Add lazy bathalign, and --nofs to skip frameshift calibration - #15
Conversation
BATH dropped HMMER's hmmalign driver but kept the whole tracealign engine (tracealign.c, p7_tracealign_*) compiled into libhmmer.a. bathalign is a 288-line driver over that existing engine -- no new alignment code. Alphabet-generic, as the engine is; BATH3/f is a superset of HMMER3/f, so it reads both .bhmm and .hmm. Amino output is byte-identical to HMMER 3.4 hmmalign across --mapali, --outformat, --trim, and Stockholm and afa output, for models from bathbuild and hmmbuild alike. Note that bathbuild itself is amino-only, so non-amino models must come from hmmbuild. c2ba68e made frameshift tau calculation unconditional and dropped --fs, but left the gating in place: evalues.c still guards FS3/FS5 on cfg_b->fs and p7_hmmfile.c still guards the header lines on hmm->fs. --nofs re-exposes that switch against the new default. Alignment never reads the frameshift stats, so for a model built to be aligned against, the calibration is pure waste -- and it is more than half of bathbuild's runtime. Where the model exists only for alignment, building it dominates the cost of aligning. 400 dipteraODB12 alignments, serial, 3 runs each, in seconds: full 43.83 / 43.32 / 43.38 nofs 15.26 / 15.50 / 15.57 A --nofs model is byte-identical to a full one apart from the omitted FS3/FS5 STATS and FRAMESHIFT PROB lines, and gives identical bathsearch results when --fs/--fsonly are not used. When they are, bathsearch fails cleanly on the unset stats rather than mis-scoring.
3a7deac to
32e4df4
Compare
|
As a further note, the MSV/Viterbi/Forward fits in p7_Calibrate() are dead weight for the same align-only case, and cost about as much again: over 400 dipteraODB12 alignments, --nofs is 15.4s and skipping the standard fits drops bathbuild runtime to 10.4s (1.5x) with bathalign producing identical alignments over the 400 tested dipteraODB12 genes. I did NOT implement this here because it is a separate and riskier change. -- p7_Calibrate() has to leave evparam unset and p7H_STATS clear, and nothing currently checks p7H_STATS, so bathsearch would score against p7_EVPARAM_UNSET instead of failing. Edit: The force push was just changing the commit description to match the edited conversation in the initial post. |
GenevieveKrause
left a comment
There was a problem hiding this comment.
Verified bathalign and the underlying --nofs mechanism thoroughly -- built both branches, confirmed a --nofs model is byte-identical to a full one apart from the three FS3/FS5/FRAMESHIFT PROB lines, confirmed bathsearch gives identical results on both when --fs isn't used, confirmed bathsearch --fs fails cleanly (not a crash, not a mis-score) on a --nofs model, and confirmed bathconvert correctly repairs a --nofs model into a full one on demand. bathalign itself works correctly across --mapali, --trim, --outformat, and both .bhmm/.hmm input. Full build + make check (126/126) pass on top of current main. Nice work -- this is genuinely time-consuming to verify and it all held up.
One design request before merging, though: bathsearch treats frameshift search as opt-in (--fs turns it on, off by default). This PR makes bathbuild's frameshift calibration opt-out instead (--nofs turns it off, on by default) -- so a plain bathbuild still pays the full calibration cost (the majority of runtime per your own numbers) for a feature that's off by default at search time. Given bathconvert already recomputes FS3/FS5 on demand if they're missing (I tested this -- it works), the recovery cost of defaulting to "off" seems low. Would you be open to flipping this to match bathsearch's convention -- an opt-in --fs flag on bathbuild (default off) instead of opt-out --nofs? Reusing the --fs name specifically would also make the mental model consistent across both tools. Happy to make this change myself if you'd rather not touch it, but wanted to raise it as a question first rather than just pushing something to your branch.
Smaller, related item: the current --nofs help text ("skip frameshift stats; model is align-only, not searchable") and the build-summary line ("frameshift stats calculated: NO (align-only model)") overstate the limitation -- a model built this way works completely normally with ordinary bathsearch; it's only bathsearch --fs/--fsonly that's unavailable. Whichever way the flag question above lands, this wording should get corrected to something like "won't support bathsearch --fs/--fsonly" rather than "not searchable."
--fs is back, opt-in, default off. Partial revert of c2ba68e: same option name and default as before April, keeping that commit's bathconvert/bathfetch work, which lets a model get FS3/FS5 added later instead of rebuilt. Help text and summary line fixed too. This changes the default on main. Anyone doing bathbuild then bathsearch --fs will need to add --fs or run bathconvert. i9-optional-annotation.pl built a model without --fs and then searched it with --fs; it now builds with --fs. bathsearch had three --fs checks all saying "not formated for this version bathsearch". Under the new default that error usually just means the model was built without --fs, so I merged them into one check that says what's missing and how to fix it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
7ae0b35 to
1ae94c1
Compare
|
Thanks for the thorough review, and for the feedback!
Worth stating that this changes the default on Bathsearch had three One testsuite fix came with it: PS: Just in case this was missed: TravisWheelerLab/easel#1. |
GenevieveKrause
left a comment
There was a problem hiding this comment.
Verified — rebuilt from scratch (autoreconf/configure/make), full make check passes including the updated i9-optional-annotation.pl.
Hands-on checked all three changes:
bathbuildwith no--fsnow reports "frameshift stats calculated: NO" — opt-in, default off, matchingbathsearch's convention. ✅- The old "align-only, not searchable" text is gone.
bathsearch --fson a plain-built model now gives one clear message:HMM file plain.bhmm has no frameshift statistics, which --fs/--fsonly require. Rebuild with 'bathbuild --fs', or add them with 'bathconvert new_file.bhmm plain.bhmm'.— confirmed both remedies actually work (bathbuild --fsfrom scratch, andbathconvertrepairing an existing model), andbathsearch --fssucceeds afterward either way.
On the consolidated error check: yes, keep it — it's strictly more informative than the three duplicate "not formatted for this version" messages it replaces, and I confirmed it fires correctly.
Good call deferring the p7_Calibrate() dead-weight optimization — agreed that's a separate, riskier change since nothing currently checks p7H_STATS. Thanks also for flagging the Easel PR, I'll take a look separately.
Merging via squash.
BATH dropped HMMER's hmmalign driver but kept the whole tracealign engine (tracealign.c, p7_tracealign_*) compiled into libhmmer.a. bathalign is a 288-line driver over that existing engine -- no new alignment code. Alphabet-generic, as the engine is; BATH3/f is a superset of HMMER3/f, so it reads both .bhmm and .hmm. Amino output is byte-identical to HMMER 3.4 hmmalign across --mapali, --outformat, --trim, and Stockholm and afa output, for models from bathbuild and hmmbuild alike. Note that bathbuild itself is amino-only, so non-amino models must come from hmmbuild.
c2ba68e made frameshift tau calculation unconditional and dropped --fs, but left the gating in place: evalues.c still guards FS3/FS5 on cfg_b->fs and p7_hmmfile.c still guards the header lines on hmm->fs. --nofs re-exposes that switch against the new default.
Alignment never reads the frameshift stats, so for a model built to be aligned against, the calibration is pure waste -- and it is more than half of bathbuild's runtime. Where the model exists only for alignment, building it dominates the cost of aligning.
400 dipteraODB12 alignments, serial, 3 runs each, in seconds:
full 43.83 / 43.32 / 43.38
nofs 15.26 / 15.50 / 15.57
A --nofs model is byte-identical to a full one apart from the omitted FS3/FS5 STATS and FRAMESHIFT PROB lines, and gives identical bathsearch results when --fs/--fsonly are not used. When they are, bathsearch fails cleanly on the unset stats rather than mis-scoring.