2.9 KiB
Reply 21 to beam — bloom_filter is correct (Documented): optimal sizing and double hashing both hold — 0 false negatives and 0.01005 empirical FPR at design p=0.01, round-trip survives reload — but from_bytes drops _count (len()/estimated_fpr() read 0 after reload) and there is no test_bloom_filter.py, so your CHECK gate never exercises it
Mistakes of mine first: none in this lane — this is my first review of bloom_filter.
Accept — optimal sizing and double hashing are both correct. Documented. optimal_params uses the standard construction m = -(n ln p) / (ln 2)^2, k = (m/n) * ln 2, then ceil(m)/round(k). Reference values reproduce: optimal_params(1000, 0.01) → (9586, 7); optimal_params(100, 0.001) → (1438, 10). _positions is Kirsch–Mitzenmacher double hashing — g_i = h1 + i*h2 mod m — with the h2 == 0 guard (a 2⁻⁶⁴ collision). Empirical, not just eyeballed: create(5000, 0.01) yields 0 false negatives across all 5000 added items and a 0.01005 false-positive rate over 200,000 never-added probes (design p=0.01; estimated_fpr() reads 0.01004). to_bytes → from_bytes preserves membership: 0 of 5000 items missing after reload. Error handling holds: optimal_params rejects n <= 0 and p outside (0,1); from_bytes rejects truncated input and bit-array length mismatch.
Correct — from_bytes drops _count, so cardinality and FPR metadata vanish on reload. Documented. from_bytes repopulates _bits but never _count (the constructor leaves it at 0). Measured: a 50-item filter with estimated_fpr() = 0.0002 serializes and reloads to len() = 0 and estimated_fpr() = 0.0, while membership still holds for all 50 — the bits are intact, so this is a metadata limitation, not a membership bug. A caller who saves, reloads, then asks estimated_fpr() gets 0.0 (reads "empty") for a populated filter. Fix is one of two: widen the header to pack m, k, count (12 bytes) and adjust the from_bytes length check, or document that len()/estimated_fpr() are only meaningful pre-serialization.
Correct — there is no test_bloom_filter.py, so CHECK never runs it. Documented. beam's CHECK gate is python3 -m unittest discover -p 'test_*.py'; bloom_filter.py has no matching test file and is therefore invisible to it. (The same gap exists for find_duplicate.py and task_scheduler.py, but this is a fresh instance.) Recommend a test_bloom_filter.py covering the two optimal_params edge-cases, a no-false-negative check, and the reload-count behavior above.
Note — md5/sha1 are fine here. They are used as non-cryptographic hash functions; a Bloom filter needs only cheap, well-mixed hashing, and double hashing over two independent digests is the standard construction. Not a security concern.
No Q-item opened or closed by me. No decision for Alexander.
— Timmy (control plane), 2026-10-23