Fix biased hash-list union estimate by routing it through the merger

Author: LucaCappelletti94

benches/hyperloglog_union.rs

@@ -60,6 +60,47 @@ fn bench_hyperloglog_union(c: &mut Criterion) {
     group.finish();
 }
 
-criterion_group!(benches, bench_hyperloglog_union);
+type HLL14 = HyperLogLog<Precision14, Bits6, <Precision14 as PackedRegister<Bits6>>::Array, XxHash64>;
+
+/// Builds the largest counter that is still in hash-list mode (stops just before the
+/// insertion that would convert it to a fully-fledged HyperLogLog).
+fn build_full_hash_list(seed: u64) -> HLL14 {
+    let mut hll = HLL14::default();
+    for value in iter_random_values::<u64>(1_000_000, None, Some(seed)) {
+        let mut candidate = hll.clone();
+        candidate.insert(&value);
+        if !candidate.is_hash_list() {
+            break;
+        }
+        hll = candidate;
+    }
+    assert!(hll.is_hash_list());
+    hll
+}
+
+/// Compares the two ways of estimating a union cardinality when both operands are large
+/// hash lists (the regime where inclusion-exclusion is biased): the current
+/// inclusion-exclusion path versus the merger-based estimate.
+fn bench_union_estimate(c: &mut Criterion) {
+    let mut group = c.benchmark_group("union_estimate_hash_list_p14");
+
+    let left = build_full_hash_list(0x00A1_1CE0);
+    let right = build_full_hash_list(0x0000_B0B0);
+    assert!(left.is_hash_list() && right.is_hash_list());
+
+    // Current: inclusion-exclusion (fast, biased low in this regime).
+    group.bench_function("inclusion_exclusion", |b| {
+        b.iter(|| black_box(black_box(&left).estimate_union_cardinality(black_box(&right))));
+    });
+
+    // Proposed: build the merged counter and estimate its cardinality (accurate).
+    group.bench_function("merger", |b| {
+        b.iter(|| black_box((black_box(&left) | black_box(&right)).estimate_cardinality()));
+    });
+
+    group.finish();
+}
+
+criterion_group!(benches, bench_hyperloglog_union, bench_union_estimate);
 
 criterion_main!(benches);

src/hyperloglog.rs

@@ -475,48 +475,19 @@ impl<P: Precision, B: Bits, R: Registers<P, B>, H: HasherType> HyperLogLog<P, B,
     ) -> f64 {
         match (self.is_hash_list(), other.is_hash_list()) {
             (true, true) => {
-                let left_hash_bits = self.get_hash_bits().unwrap();
-                let right_hash_bits = other.get_hash_bits().unwrap();
-                assert!(left_hash_bits >= GapHash::<P, B>::SMALLEST_VIABLE_HASH_BITS);
-                assert!(right_hash_bits >= GapHash::<P, B>::SMALLEST_VIABLE_HASH_BITS);
-
-                let left_shift = if left_hash_bits <= right_hash_bits {
-                    0
-                } else {
-                    left_hash_bits - right_hash_bits
-                };
-                let right_shift = if right_hash_bits <= left_hash_bits {
-                    0
-                } else {
-                    right_hash_bits - left_hash_bits
-                };
-
-                let left_hashes = self.registers.as_ref();
-                let right_hashes = other.registers.as_ref();
-                let left_bit_index = self.get_writer_tell();
-                let right_bit_index = other.get_writer_tell();
-
-                let intersection_cardinality = f64::from(intersection_from_sorted_iterators(
-                    GapHash::<P, B>::downgraded(
-                        left_hashes,
-                        self.get_number_of_hashes().unwrap(),
-                        left_hash_bits,
-                        left_bit_index,
-                        left_shift,
-                    ),
-                    GapHash::<P, B>::downgraded(
-                        right_hashes,
-                        other.get_number_of_hashes().unwrap(),
-                        right_hash_bits,
-                        right_bit_index,
-                        right_shift,
-                    ),
-                ));
-
-                let union_cardinality =
-                    self_cardinality + other_cardinality - intersection_cardinality;
-
-                correct_union_estimate(self_cardinality, other_cardinality, union_cardinality)
+                // Build the union as a hash list and estimate its cardinality directly, so the
+                // birthday-paradox correction is applied to the union the same way it is to a
+                // single counter. Inclusion-exclusion (A + B - intersection) would subtract a
+                // raw, uncorrected count of coinciding downgraded hashes; for sets with little
+                // real overlap those coincidences are dominated by spurious birthday collisions,
+                // which biases the union estimate low and increasingly so at higher precisions.
+                let mut union = self.clone();
+                union.merge(other);
+                correct_union_estimate(
+                    self_cardinality,
+                    other_cardinality,
+                    union.estimate_cardinality(),
+                )
             }
             (true, false) => {
                 let hash_bits = self.get_hash_bits().unwrap();

src/utils.rs

@@ -9,15 +9,13 @@
 
 mod constants;
 mod hasher_type;
-mod intersection_from_sorted_iterators;
 mod matrix;
 mod number;
 mod random;
 mod variable_word;
 
 pub use constants::*;
 pub use hasher_type::HasherType;
-pub(crate) use intersection_from_sorted_iterators::intersection_from_sorted_iterators;
 pub use matrix::Matrix;
 pub(crate) use number::{FloatOps, Number, PositiveInteger};
 pub use random::*;