Add protein and nucleotide profiles #198
Conversation
|
@h-2 Do we want tests for the different profiles? I didn't think it was necessary given that we also don't test all other parameters so I did not add them but let me know if you think differently |
src/search_options.hpp
Outdated
| int32_t minBitScore = 42; | ||
| double maxEValue = -1; | ||
| int32_t idCutOff = 0; | ||
| uint64_t maxMatches = 256; |
There was a problem hiding this comment.
This means you need to recreate the search test files I am afraid...
| { | ||
| if (options.nucleotide_mode) | ||
| { | ||
| options.iterativeSearch = false; |
There was a problem hiding this comment.
I thought we came to the conclusion that the iterativeSearch is always beneficial, or did we not find a good parameter-pair for nucleotide+fast?
There was a problem hiding this comment.
We did not find a good pair and you suggested to pick one of the search0 candidates as regular search for fast.
src/search_options.hpp
Outdated
| int32_t minBitScore = 42; | ||
| double maxEValue = -1; | ||
| int32_t idCutOff = 0; | ||
| uint64_t maxMatches = 256; |
There was a problem hiding this comment.
This means you need to recreate the search test files I am afraid...
src/search_options.hpp
Outdated
| // "Size of the DP-band used in extension (-3 means log2 of query length;" | ||
| // " -2 means sqrt of query length; -1 means full dp; n means band of size 2n+1)", | ||
| // seqan3::option_spec::advanced, | ||
| // seqan3::arithmetic_range_validator{-3, 1000}); |
There was a problem hiding this comment.
If nothing uses this, I would propose to remove it (also the option itself). No point in carrying around more legacy codepaths.
There was a problem hiding this comment.
I guess depends if we ever want to introduce it again, that's why I left it. But we can remove it, just wanted to check.
There was a problem hiding this comment.
If we add it again, we can also add the option back. Considering other people (including future-me), I think it's better to reduce the amount of commented-out code!
I don't think it would hurt to have at least some tests for the other profiles. Even if we don't test all options/combinations, testing the advertised ones gives us a better coverage. |
Should we test |
Sounds good! |
sarahet
force-pushed
the
protein_modes
branch
from
3b3faab to
016a335
Compare
3 participants
This PR includes #194 and #191 and adds nucleotide profiles as well as updated tests matching changes in index and the search default settings.