Closed form approximations#136
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the diff-count one-bit calibration lookup/newton-on-summation path with a closed-form approximation (and a cheap Newton refinement on that approximation) to estimate set size from observed one-bit counts. It also documents how the metric’s error grows with larger set differences and provides guidance on configuration choice.
Changes:
- Add closed-form forward approximation for
expected_diff_bucketsand a corresponding inverse (estimate_count_approx) with Newton refinement. - Switch
OnesMetric::{to_f32, from_f32}to use the new approximations instead of the exact-model inversion. - Expand documentation on precision/error growth and recommended configs (
b >= 10) in both rustdoc and README.
Show a summary per file
| File | Description |
|---|---|
| crates/geo_filters/src/diff_count/metric.rs | Uses the new closed-form estimation functions for metric calibration and adds guidance in rustdoc. |
| crates/geo_filters/src/diff_count/config.rs | Implements the closed-form forward approximation and inverse (Newton refinement) plus tests comparing against the exact model. |
| crates/geo_filters/README.md | Adds detailed explanation of relative error growth and configuration guidance for nearest-neighbor ranking. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Low
Comment on lines
+162
to
+176
| let s = 0.5 / -phi.ln(); | ||
| let a = 2.0 * (1.0 - phi); | ||
| // Invert the large-N asymptote `m ≈ S (gamma + ln(a N)) + 1/4` for the initial guess. | ||
| let mut items = ((ones - 0.25) / s - GAMMA).exp() / a; | ||
| if !items.is_finite() || items <= 0.0 { | ||
| items = ones; | ||
| } | ||
| for _ in 0..3 { | ||
| let (m, dm) = expected_diff_buckets_approx(phi, items); | ||
| items -= (m - ones) / dm; | ||
| if items < f64::MIN_POSITIVE { | ||
| items = f64::MIN_POSITIVE; | ||
| } | ||
| } | ||
| items |
Comment on lines
+124
to
+126
| /// reorder candidates whose true distances are within roughly a factor of two of each other, | ||
| /// returning an approximate rather than exact nearest neighbor; for an exact result, shortlist with | ||
| /// this metric and re-rank the top candidates with [`Count::size_with_sketch`](crate::Count). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a closed form approximation of the size estimation.
For the inverse, it was simpler to use Newton than trying to find another approximating function.
Note: these functions are used instead of the LUT table, since the LUT only works for a certain number of one bits.
The PR also adds documentation how the error rate increases for the metric when the set size goes up and a guideline when to use it.