feat/better seed debug logging #1240
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
corneliusroemer
force-pushed
the
feat/better-seed-debug-logging
branch
from
05c958c to
27d669e
Compare
| let max_shift = seed_matches.iter().map(|sm| sm.offset.abs()).max().unwrap_or(0); | ||
| // Need to iterate in pairs | ||
| let max_unmatched_ref_stretch = seed_matches | ||
| .iter() | ||
| .tuple_windows() | ||
| .map(|(sm1, sm2)| sm2.ref_pos - (sm1.ref_pos + sm1.length)) | ||
| .max() | ||
| .unwrap_or(0); | ||
| let max_unmatched_qry_stretch = seed_matches | ||
| .iter() | ||
| .tuple_windows() | ||
| .map(|(sm1, sm2)| sm2.qry_pos - (sm1.qry_pos + sm1.length)) | ||
| .max() | ||
| .unwrap_or(0); | ||
| let first_match = seed_matches.first().unwrap(); | ||
| let last_match = seed_matches.last().unwrap(); | ||
| let first_match_ref_pos = first_match.ref_pos; | ||
| let first_match_qry_pos = first_match.qry_pos; | ||
| let last_match_ref_pos = ref_seq.len() - last_match.ref_pos - last_match.length; | ||
| let last_match_qry_pos = qry_seq.len() - last_match.qry_pos - last_match.length; | ||
|
|
||
| let n_matches = seed_matches.len(); | ||
| debug!("Chained seed stats. max indel: {max_shift}, # matches: {n_matches}, first/last match distance from start/end [start: (ref: {first_match_ref_pos}, qry: {first_match_qry_pos}), end: (ref: {last_match_ref_pos}, qry: {last_match_qry_pos})], max unmatched stretch (ref: {max_unmatched_ref_stretch}, qry stretch: {max_unmatched_qry_stretch})"); |
There was a problem hiding this comment.
Could you please extract the body of the condition into a separate function (keeping the condition itself where it is), so that it does not distract from the main algo code.
Note that the conditional jump will always be executed in production. (And every log macro invocation also adds a similar conditional jump underneath, which might hurt performance in tight loops. This particular place seems to be executed rarely though.)
Is this thing required in release builds?
There was a problem hiding this comment.
Good point to refactor into a function.
I know that this will be always executed in production - but it's only executed once per sequence, I don't think we need to think about perf at things so rarely executed.
Not required in release builds but it's nice if once can always get this info - we also include lots of trace statements in release builds. What's the argument against including it?
There was a problem hiding this comment.
Okay. No particular argument other than it becomes unnecessary instructions for most executions. Let's hope branch predictor figures it out :)
ivan-aksamentov
force-pushed
the
master
branch
2 times, most recently
from
93f31a9 to
edefa32
Compare
ivan-aksamentov
force-pushed
the
master
branch
from
27d4a31 to
9010d85
Compare
We're currently not really using
debuglevel logging for anything. I think it'd be useful to output some basic stats for each sequence like what proportion of the query are covered by chained matches, mean band width, max indel, and things like that.This is what this PR outputs at debug level (
-vv):