Conversation
They are now used to directly set w_min and w_max instead of the previous (confusing) interpretation. The names w_min and w_max are still used in the code at the moment, mostly in order to avoid doing too many things at the same time, but also because w_min and w_max is a bit easier to read than l and u. This will be confusing for existing scripts!
| , l{parser, "INT", "Lower syncmer offset from k/(k-s+1). Start sample second syncmer k/(k-s+1) + l syncmers downstream [0]", {'l'}} | ||
| , u{parser, "INT", "Upper syncmer offset from k/(k-s+1). End sample second syncmer k/(k-s+1) + u syncmers downstream [7]", {'u'}} | ||
| , k{parser, "INT", "Syncmer (strobe) length, has to be below 32. [20]", {'k'}} | ||
| , l{parser, "INT", "Start of sampling window for second syncmer (i.e., second syncmer must be at least l syncmers downstream). [5]", {'l'}} |
There was a problem hiding this comment.
I think this should be 4 as the default value? 20/(20 (20-16+1) + 0 = 4.
There was a problem hiding this comment.
Yes, but the help text was wrong: The actual default in main for the 150 nt profile is -l 1, not -l 0.
I wanted to avoid making mistakes here, so I ran strobealign with all the canonical read lengths and looked at the log output to get the correct w_min and w_max values. Maybe I should run one last test to verify that they are the same now after the change.
| Profile{ 75, 90, 20, -4, 1, 6}, | ||
| Profile{100, 110, 20, -4, 2, 6}, | ||
| Profile{125, 135, 20, -4, 3, 8}, | ||
| Profile{150, 175, 20, -4, 5, 11}, |
There was a problem hiding this comment.
Profile{150, 175, 20, -4, 5, 11}, should then be Profile{150, 175, 20, -4, 4, 11}, The error could have propagated to other profiles (didn't check).
|
Since my comments are likely wrong and you have done a check I approve. |
2 participants
They are now used to directly set w_min and w_max instead of the previous (confusing) interpretation.
The names
w_minandw_maxare still used in the code at the moment, mostly in order to avoid doing too many things at the same time, but also becausew_minandw_maxis a bit clearer thanlandu.Note that this will be confusing for existing scripts; they need to be adjusted.