feat(theta): add jaccard similarity#142
Conversation
| } | ||
| } | ||
|
|
||
| fn compute_union<A: ThetaSketchView, B: ThetaSketchView>( |
There was a problem hiding this comment.
Looks like we better have union implementation here first.
| } | ||
| } | ||
|
|
||
| fn approximate_lower_bound_on_p(n: u64, k: u64, num_std_devs: f64) -> f64 { |
There was a problem hiding this comment.
These are general math function which can be place in bounds_binomial_proportions.rs
There was a problem hiding this comment.
Addressed by moving the general binomial proportion helpers into common::bounds_binomial_proportions; jaccard_similarity.rs now calls approximate_lower_bound_on_p / approximate_upper_bound_on_p from that common module.
|
We also miss |
tisonkun
left a comment
There was a problem hiding this comment.
We must not have a unit struct ThetaJaccardSimilarity for creating JaccardSimilarity.
Let me consider the API and review the helper functions location.
Typically, here are a few API issues that should be resolved:
- Should have
JaccardSimilarity::between/JaccardSimilarity::between_with_seed, rather thanThetaJaccardSimilarity::jaccard/ThetaJaccardSimilarity::jaccard_with_seed. JaccardSimilarityshould expose its internal fields with getter, not pub fields, because the fields are immutatble.
…/datasketches-rust into feat/theta-jaccard-similarity
|
Addressed the API feedback from @tisonkun:
The tests have been updated to use the new API. |
…to feat/theta-jaccard-similarity
Summary
ThetaJaccardSimilarityfor Theta sketches.theta_jaccard_similarity_test.cppcases.Motivation
This closes one small P0 parity gap with
apache/datasketches-cpp: Rust had Theta sketching and intersection support, but no Jaccard similarity API.Implementation Notes
ThetaIntersectionfor the intersection side.ThetaUnionAPI in this PR.Tests
cargo check -p datasketches --features thetacargo test -p datasketches --features theta --test theta_jaccard_similarity_testcargo test -p datasketches --features theta --lib --test theta_intersection_test --test theta_jaccard_similarity_test --test theta_sketch_testFull
cargo test -p datasketches --features thetacurrently reaches unrelated failures intheta_serialization_testbecause localdatasketches/tests/serialization_test_data/...files are missing.