[WIP] Fix MinHash merge
of abund + noabund objects (rust).
#1395
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.
KmerMinHash::merge
insrc/core/src/sketch/minhash.rs
is leaving the abundance vectors in an inconsistent state when merging two MinHash objects, one with track_abundance=True and one without. This PR adds tests for several but not all of the conditions that aren't being handled, and also proposes some (probably incorrect) Rust-level code fixes.I think a better solution might be to simply declare this kind of merge to be not allowed, and to force people to flatten the abund MinHash if they want to merge with a no-abund MinHash.
@luizirber your input solicited!
Checklist
make test
Did it pass the tests?make coverage
Is the new code covered?without a major version increment. Changing file formats also requires a
major version number increment.
changes were made?