jaideeppyne commented on issue #460:
URL:
https://github.com/apache/datasketches-cpp/issues/460#issuecomment-5466837216
I ran a differential test between the two implementations to chase this down.
Oracle: Java and C++ are specified to serialize identically, so any
disagreement is a bug in one of them. I built datasketches-cpp at `c8e4218`
(master) and datasketches-java at `d5cce9b` (main, 9.1.0-SNAPSHOT), generated
the same 20 theta cases on both sides (empty, single item, exact, estimation,
sampling with p<1, union, intersection, a-not-b, ordered and unordered, plus
the v4 compressed format) and compared serialized bytes along with retained
count, theta, estimate and both bounds. 11 of 20 are byte identical. The 9 that
differ do so in exactly two places, and there is a third finding that is not
about bytes at all.
**1. The `p` bytes you originally reported are already fixed.** Java commit
8044090 ("Set P to zero for compact sketches", Aug 2025) changed
`insertP(dstWSeg, (float) 1.0)` to `(float) 0.0` in `CompactOperations`, with
the comment "0.0 to be consistent with C++". That shipped in datasketches-java
9.0.0; @nmahadevuni was on 5.0.1. On current main the exact 2-item case from
the first post now matches byte for byte on both sides:
```
02 03 03 00 00 1a cc 93 02 00 00 00 00 00 00 00
15 f9 7d cb bd 86 a1 05 c3 97 fc 12 81 70 9d 1e
```
**2. The single item flag is still there**, in 3 of my 20 cases. Java sets
bit 5, C++ does not. It is slightly more than cosmetic, because Java's own
`PreambleUtil.preambleToString` derives the count from that flag (`int curCount
= singleItem ? 1 : 0`). So Java's preamble dump of a C++ single item sketch
reports CurrentCount 0 for a sketch that holds 1 entry:
```
cpp bytes 01 03 03 00 00 1a cc 93 | 15 f9 7d cb bd 86 a1 05
Java preambleToString -> CurrentCount: 0, SINGLE_ITEM: false (actual
retained: 1)
Java on its own bytes -> CurrentCount: 1, SINGLE_ITEM: true (actual
retained: 1)
```
Java's real parser gets it right: `SingleItemSketch.checkForSingleItem`
masks bit 5 off and accepts `(flags & 0x1F) == 0x1A`, with the comment
"SingleItem flag may not be set due to a historical bug, so we can't depend on
it for now". So Java's diagnostic path contradicts Java's own parser,
independent of what C++ does. Whichever way you decide to align the bytes, that
`preambleToString` line looks worth fixing on its own.
**3. A third divergence that is not in this issue, and this one actually
bites.** For every empty compact sketch, Java writes a seed hash of `00 00` and
C++ writes `computeSeedHash(seed)`:
```
cpp : 01 03 03 00 00 1e cc 93
java: 01 03 03 00 00 1e 00 00
```
That accounts for the other 6 differing cases. Java is deliberate here:
`EmptyCompactSketch` is a singleton constant `{1, 3, 3, 0, 0, 0x1E, 0, 0}` and
the source says "The seedHash is ignored".
C++ mostly agrees that an empty sketch's seed hash is meaningless.
`deserialize_v3` and `deserialize_v4` do `if (!is_empty) check_seed_hash(...)`,
`theta_union_base::update` returns early on `sketch.is_empty()`, and
`theta_intersection_base::update` does `if (!sketch.is_empty() && ...)`. All
three existing seed mismatch tests carry the comment "non-empty should not be
ignored".
`theta_set_difference_base::compute` is the one path that does not, and it
throws. Its early return only fires when `a.get_num_retained() > 0`, so when A
is non-empty with zero retained entries the check is reached. Reproducible in
C++ alone, no Java needed:
```cpp
auto b = update_theta_sketch::builder().set_seed(12345).build().compact();
// empty, foreign seed
auto a = update_theta_sketch::builder().set_p(1e-6f).build();
// non-empty, 0 retained
a.update(1); a.update(2); a.update(3);
compact_theta_sketch::deserialize(...) // ACCEPTED
theta_union(seed=9001).update(b) // ACCEPTED
theta_intersection(seed=9001).update(b) // ACCEPTED
theta_a_not_b(seed=9001).compute(a, b) // THREW -> B seed hash mismatch
```
Cross language, the same thing happens with an unmodified Java empty sketch
as B, in all 6 of my empty cases, because Java's seed hash of 0 never matches.
Fix is one line, guarding B's check with `!b.is_empty()` the way intersection
already does. I opened #517 for it: all 17 ctest suites pass and the 6 cross
language failures go to 0.
So to answer the original question directly: the `p` difference was a Java
bug and is fixed, the single item flag is still unaligned and exposes a real
inconsistency inside Java, and the empty sketch seed hash difference is
intentional in Java but exposes a real bug in C++ a-not-b.
Happy to share the 20 case harness if it is useful for the stricter byte
comparison @AlexanderSaydakov mentioned above; it is about 100 lines on each
side.
I used Claude Code to help run the differential harness and draft this
comment. I verified every claim above against actual build and test output.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
