solo: --soloUMIfiltering MultiGeneUMI_All (closes #144) - #152
Open
BenjaminDEMAILLE wants to merge 3 commits into
Open
solo: --soloUMIfiltering MultiGeneUMI_All (closes #144)#152BenjaminDEMAILLE wants to merge 3 commits into
BenjaminDEMAILLE wants to merge 3 commits into
Conversation
BenjaminDEMAILLE
changed the title
BenjaminDEMAILLE
changed the title
BenjaminDEMAILLE
changed the title
BenjaminDEMAILLE
changed the title
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 29, 2026
CONTRIBUTING.md requires the description to match the code; the entries for the other themes split out of scverse#152 belong to their own PRs.
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 29, 2026
CONTRIBUTING.md requires the description to match the code; the entries for the other themes split out of scverse#152 belong to their own PRs.
…GeneUMI `--soloUMIfiltering MultiGeneUMI_All` resolved to the same variant as `MultiGeneUMI`, which is neither what STAR does nor what the option is documented to do. Of the three available behaviours it was the only one nobody had asked for. In STAR the option is a no-op: it is parsed and stored, but its consumption site tests only the `MultiGeneUMI` flag, so selecting it leaves the filter entirely off. Documented, it removes a UMI seen in more than one gene from *all* of them, rather than from the losers only. `UmiFiltering::MultiGeneUmiAll` now exists and does the documented thing: a UMI appearing in several genes is evidence of a collision or of chimeric amplification, so it is discarded outright rather than attributed to whichever gene happened to read deepest. Single-gene UMIs are untouched, which the test checks across every mode. Raised upstream as scverse#144 before changing it, since "be faithful to STAR" and "do what the flag says" genuinely point in opposite directions here. Also adds `docs-old/dev/divergences.md`, recording this and the homopolymer-UMI rule, so deliberate differences are written down rather than rediscovered as surprises in a differential run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
force-pushed
the
bd/solo-umi-filter
branch
from
July 29, 2026 09:33
5b4456e to
1383b87
Compare
BenjaminDEMAILLE
changed the title
The homopolymer-UMI rule describes behaviour already on main, not something this diff changes, and its locking test does not exist yet. It belongs in its own change rather than riding along here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Jul 30, 2026
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
1 participant
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.
--soloUMIfiltering MultiGeneUMI_Alldoes what it is documented to do. Closes #144.What changed
UmiFiltering::MultiGeneUmiAllbecomes its own variant instead of an alias, and removes a UMI seen in more than one gene from all of them.MultiGeneUMIflag, so selecting it leaves the filter offMultiGeneUMI: keep the deepest-read geneWhy
Reproducing the no-op ships a flag that silently does nothing to anyone who read STAR's documentation. #144 records the choice, and it was raised there before any code changed, because "be faithful to STAR" and "do what the flag says" genuinely point in opposite directions here.
Divergence
DIVERGENCE.md§1.2, with the STAR source checked (SoloFeature_collapseUMIall.cpp,ParametersSolo.cpp). Needs maintainer sign-off per CONTRIBUTING. Inverting it is a one-line change, since the test asserts the behaviour rather than the direction.Verification
multigene_umi_all_drops_the_umi_from_every_gene— a UMI seen in two genes disappears from both, whereMultiGeneUMIkeeps the deeper onemultigene_umi_all_parses_to_its_own_variant— it is no longer an aliasGate: 562 lib + 26 integration tests,
cargo clippy --all-targets -- -D warnings,cargo fmt --check, MSRV 1.89 — all green.This is a counting change on the
MultiGeneUMI_Allpath only. The default (-) and the other modes produce identical output, so no existing run changes.Not run:
test/solo_diff_docker.sh. Since the default is unaffected, the harness would compare an unchanged path; the changed path has no STAR behaviour to compare against, because in STAR it does nothing. Sign-off on the divergence is what this needs rather than an oracle run.This PR was previously bundled with four other solo themes; those are now #156 (EmptyDrops_CR statistics), #157 (CellReads.stats), #158 (Transcript3p) and #159 (soloCellFiltering), per the one-theme rule in CONTRIBUTING.md.