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

revisit LinearIndex.select behavior for scaled (and num?) #1433

Closed
ctb opened this issue Apr 2, 2021 · 1 comment
Closed

revisit LinearIndex.select behavior for scaled (and num?) #1433

ctb opened this issue Apr 2, 2021 · 1 comment

Comments

@ctb
Copy link
Contributor

ctb commented Apr 2, 2021

in this review, @bluegenes pointed out that LinearIndex.select(...) was not necessarily correct -

I like the approach of selecting sigs that could be converted! ...but not all scaled sigs can be downsampled (desired scaled < sig scaled), right?

and I said I'd put together a test. But my current hot take is that the way we do downsampling in the code base, two scaled signatures will always be downsampled to the higher scaled value, so there is never a situation where select(...) will fail on two scaled sequences. This is particular to LinearIndex and other signature collections, though; SBT and LCA indices cannot always be downsampled.

anyway, this is something I want to revisit! maybe as part of more/better tests, #1427.

@ctb
Copy link
Contributor Author

ctb commented Aug 3, 2022

from #1524,

A few design decisions were made as part of this - the most consequential one is that select just selects compatible signatures, and doesn't actually do any downsampling or anything. See #1072 (comment) for links.

This has proven to be a good design decision, I think, because we have situations where we do or may want access to the original sketch. Closing this issue.

@ctb ctb closed this as completed Aug 3, 2022
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