Skip to content
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

update 'num' behavior, in terms of downsampling and comparison checking #870

Open
ctb opened this issue Jan 27, 2020 · 3 comments
Open

update 'num' behavior, in terms of downsampling and comparison checking #870

ctb opened this issue Jan 27, 2020 · 3 comments

Comments

@ctb
Copy link
Contributor

ctb commented Jan 27, 2020

see #856 (comment) - there,

ctb

in src/core/src/sketch/minhash.rs, the if ignore_abundance code in similarity seems duplicative with the compare function; this should be resolved in some way, or at least commented.
is there a reason not to have SourmashError::MismatchNum be checked in check_compatible (used by lib.kmerminhash_compare)?

luizirber:

These two are related: when I made #808 I had SourmashError::MismatchNum in check_compatible and merged similarity and compare, but that broke the code because check_compatible was not used in any of the scaled/downsampled parts of the code (which obviously don't have the same num.

@ctb
Copy link
Contributor Author

ctb commented Jan 28, 2020

well, good news is that when you add MismatchNum checking in check_compatible, you only break 6 tests, all in test__minhash.py. Not sure if that's because we're not testing different 'num' minhashes from the command line, or what!

@ctb
Copy link
Contributor Author

ctb commented Jan 28, 2020

...looks like we're not testing it via the command line 😆

@ctb
Copy link
Contributor Author

ctb commented Apr 20, 2022

#1955 adds in some code for this downsampling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant