jaideeppyne opened a new pull request, #239: URL: https://github.com/apache/datasketches-rust/pull/239
`HllUnion::update` builds its gadget through `copy_or_downsample`, which rebuilds the registers with a bulk merge. That marks the estimator out of order and zeroes the HIP accumulator. The code writes the source HIP back right after, but never restores the flag, so the value it just restored is dead: `estimate()` takes the composite branch and the bounds use the wider out-of-order RSE tables. Java and C++ set both fields from the source. `copy_or_downsample` in `HllUnion-internal.hpp` even comments "both of these are required for isomorphism". So in the reference, `union.update(a)` with `lg_max_k >= a.lg_k` reproduces `a` exactly. In Rust it did not. ### How I found it Same cross-language differential setup as #237, but comparing the set operations themselves rather than their results. I serialized Rust sketches, ran the *same* union in the C++ core through the pip `datasketches` package, and diffed estimate, lower and upper bounds at 1/2/3 std dev, theta, retained count and mode. That distinction is what surfaced this: a union result is internally self-consistent, so every round-trip check I ran passed happily while the operation was producing the wrong estimator. Numbers I measured, over lg_k 4 to 21, all three HLL types, n from 1 to 100k: - 432 single sketch unions with `lg_max_k >= lg_k`. C++ reproduces the input sketch in 432/432. Rust was 366/432 before, 432/432 after. - 768 single sketch unions including downsampling, checked against the C++ result on the same input bytes: 537/768 matched before, 729/768 after, no case that matched before stopped matching. - 126 two-sketch unions: mismatches against C++ went from 40 to 2. - The stragglers are all coupon-mode sources, where HIP depends on the order coupons are visited during the merge. Registers and KxQ come out bit identical there, so that is a different thing and I left it alone. - Theta, Tuple, CPC, REQ, T-Digest, Frequent Items and the Theta/Tuple set operations stayed clean across roughly 26k comparisons, before and after. One note on the oracle: C++ 5.2.0's `hll_union` returns garbage when an input `lg_k` exceeds `lg_max_k` (unioning a 200k sketch at lg_k 16 with a 3 item sketch at lg_k 14 gives 3.0). I excluded those configurations rather than trusting them. ### One existing test changed `test_union_idempotency` asserted `union(A).estimate() == union(A, A).estimate()`. C++ does not hold that. Its second sketch-to-sketch merge sets the out-of-order flag and zeroes HIP, so C++ gives 996.181 then 988.027 for that same sketch. The old assertion passed only because Rust was dropping HIP on the first update too. I kept the A∪A intent and made it assert what the reference actually guarantees: the first union reproduces A exactly, and unions from the second onward are stable. The two new tests fail on `main` with only the source reverted. `cargo x test` passes, including the 101 cross-language snapshot tests. `cargo x lint` passes; its `hawkeye` step will not install on the pinned 1.86 toolchain here, which is unrelated to this change. I wrote the harness and this patch with Claude Code, and ran everything above myself. -- 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]
