GitHub user c-dickens added a comment to the discussion: Count-Min sketch is not compatible across languages, or across C++ standard libraries
Thanks for the detailed writeup @leerho, it certainly helps clarify the problem. Regarding the specific questions to the community. 1. I support deterministic row seeds. 2. Yes. 3. **3(b)** The bug definitely needs a fix, but within one language (and, for C++ and Python, one platform) the sketch works correctly today. 4. No objection to the version bump, and I agree with 6(d)(ii). The only overall comment I have is in reference to: > I would recommend XxHash64. We only need 64 bits and it is 2x faster than > MurmurHash3_64_128, and already in our library. The proposal uses the hash only to make the row seeds from the base seed. That runs once per sketch, for a small number of rows (usually 3, 5 or 7), so the speed gain there is negligible. If we adopt XxHash64 and bump the serial version anyway, should we also use XxHash64 to hash items in update and estimate? That is where the speed gain matters, because it runs once per row on every update. The bucket fix in 6(b) already changes the same line in each language, so the extra work is small. The sketch does not record which hash made it, so a later change would need serial version 3. For that reason, I'd argue we should make this change in this update. GitHub link: https://github.com/apache/datasketches-tck/discussions/18#discussioncomment-18864968 ---- This is an automatically sent email for [email protected]. To unsubscribe, please send an email to: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
