-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add theta sketch scalar #11153
add theta sketch scalar #11153
Conversation
7f7cfe8
to
6252a71
Compare
Codecov Report
@@ Coverage Diff @@
## master #11153 +/- ##
==========================================
- Coverage 0.11% 0.00% -0.12%
==========================================
Files 2218 2207 -11
Lines 119113 118872 -241
Branches 18021 18004 -17
==========================================
- Hits 137 0 -137
+ Misses 118956 118872 -84
+ Partials 20 0 -20
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 100 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This is pretty cool, but I'd suggest we prefix the function names with sketch types, ie |
Ahh. LOL i didn't know KLL and Tuples are actually a different sketches :-) thank you for the notes. qq: does it matter what sketch is being used? the scalar function here doesn't really know what sketch is being used, all it does is
do we need different handling for KLL or Tuple based sketches? |
Right, 'sketch' is a generic term for various probabilistic data structures. Different sketches have different binary layouts and serializers/deserializers. It would be hard to write a generic function which handles all. The one you are using in this PR is for Even if we manage to identify they sketch type from serialized form, interfaces may differ. For example Hope this makes sense. |
ok so to summarize:
feels like this should be handled by type matching instead of function name. |
6252a71
to
42f5bfc
Compare
* add theta sketch scalar functions * fix sketch name to add "theta" specifically to differentiate other sketch types --------- Co-authored-by: Rong Rong <rongr@startree.ai>
enable theta sketch test for v2 after apache#11144 and apache#11153 Co-authored-by: Rong Rong <rongr@startree.ai>
added several functions
GET_THETA_SKETCH_ESTIMATE
to extract estimate from ThetaSketchTHETA_SKETCH_DIFF
to differentiate 2 ThetaSketchTHETA_SKETCH_UNION
/THETA_SKETCH_INTERSECT
for ThetaSketch set operationssyntax examples: