Use FragGeneScanRs as gene predictor instead of FragGeneScan++ - #13
Open
bmesuere wants to merge 2 commits into
Open
Use FragGeneScanRs as gene predictor instead of FragGeneScan++#13bmesuere wants to merge 2 commits into
bmesuere wants to merge 2 commits into
Conversation
FragGeneScan++ has been archived since 2021. FragGeneScanRs is maintained, faster, and ours. The only thing standing in the way was the `-c` chunk size option, which the scripts passed and FragGeneScanRs does not accept. It only tunes FGS++'s chunked threading, so dropping it costs nothing. FragGeneScanRs also needs no training data directory, so `-r` is gone too and `-f` now takes the binary (or a directory holding it) rather than a directory that must contain both `FGSpp` and `train`. When `-f` is not given, setup looks for FragGeneScanRs on the PATH, which is where cargo, conda and the release tarballs all put it. The setup check now feeds it a real read and requires a prediction to come back, rather than only checking that the binary starts. The old check passed a single base, which FragGeneScanRs panics on. Verified end to end on testdata: fastq2fasta | FragGeneScanRs | prot2tryp | filter yields 175 predictions from the 200 reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Someone following the old instructions would have hit a confusing failure: passing an FGS++ directory to -f made the setup look for FragGeneScanRs inside it, not find it, and report that invoking FragGeneScanRs failed. umgap-setup now recognises an FGS++ install, whether -f points at the directory or at the FGSpp binary itself, and says that the predictor changed. A directory holding both binaries still picks FragGeneScanRs without complaining. umgap-analyse does the same for an existing configuration directory, which is the likelier path: users who already ran the old setup have an FGSpp symlink there and no reason to run setup again. They now get told why rather than a bare "FragGeneScanRs not found". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FragGeneScan++ has been archived since 2021. FragGeneScanRs is maintained, faster, and ours.
Two commits: the swap, and the migration messages for people coming from the old setup.
The swap
The only thing in the way was
-c, which the scripts passed and FragGeneScanRs does not accept. It only tunes FGS++'s chunked threading, so dropping it costs nothing. FragGeneScanRs also carries its own training data, so-rgoes too.That makes
-fsimpler: it now takes the binary, or a directory containing it, instead of a directory that had to hold bothFGSppandtrain. Without-f, setup looks forFragGeneScanRson thePATH, which is where cargo, conda and the release tarballs all put it.The setup check also got stricter. It now feeds a real read and requires a prediction to come back, rather than only checking that the binary starts. The old check piped in a single base:
FragGeneScanRs panics on input that short (
index out of boundsinviterbi.rs), worth reporting on that repo separately, but a check that only proves the binary starts was not worth much anyway.Migration messages
Someone following the old instructions would otherwise hit a baffling failure: passing an FGS++ directory to
-fmakes setup look forFragGeneScanRsinside it, not find it, and report that invoking FragGeneScanRs failed.umgap-setup.shrecognises an FGS++ install, whether-fpoints at the directory or at theFGSppbinary, and says the predictor changed. A directory holding both binaries still picks FragGeneScanRs without complaining.umgap-analyse.shdoes the same for an existing configuration directory. This is the likelier path: anyone who ran the old setup has anFGSppsymlink there and no reason to run setup again, so they would have got a bare "FragGeneScanRs not found" with no hint that anything changed.Verification
Detection paths:
-fas a directory,-fas a binary, found onPATH, and absent. Migration paths:-fold FGS++ directory,-f .../FGSpp, directory containing both binaries, config directory with an oldFGSppsymlink, config directory with neither.End to end on the bundled testdata:
175 predictions from the 200 reads, tryptic peptides out the far end. Both scripts pass
sh -n.Note
Split out of #12 on request. That PR keeps the unrelated README corrections; the two touch disjoint parts of the README and merge cleanly in either order, which I verified locally.