-
Notifications
You must be signed in to change notification settings - Fork 81
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
fix MinHash.merge(...)
in rust
#1446
Comments
As part of the larger "What shall we do with MinHash objects?" discussion, note that here we want to come up with tests and (hopefully simple) fixes so that things don't break. Down the road we might want to also think about splitting abund and noabund |
@ctb I'm a little confused about what exactly needs to be done here. Could you please clarify? |
hi @keyabarve the most straightforward option is to take over #1395 and fix all of the issues - basically, do systematic tests of all of the if/else branches in the Rust merge code starting in Python land, and then fix the Rust merge code to pass all of the tests. Does that make sense? |
to get started, try checking out that branch with So your task would be to fix the code in Note that you can get back the right version of
|
In #1395, I detail some fun ways to make the rust code return invalid
MinHash
objects becausemerge
does Bad Things when handed mixed pairs of abundance/non-abundance sketches.This is an issue corresponding to that PR.
The text was updated successfully, but these errors were encountered: