feat(sort): multi-reference minimizer scoring for dataset suggestion#1771
Open
ivan-aksamentov wants to merge 2 commits into
Open
feat(sort): multi-reference minimizer scoring for dataset suggestion#1771ivan-aksamentov wants to merge 2 commits into
ivan-aksamentov wants to merge 2 commits into
Conversation
…mizer hits Dataset suggestion divides matched minimizers by a per-dataset denominator. For a dataset built from several references the index stores the union of their minimizers, so a union-sized denominator grows with every reference added while a query still only matches one reference's worth, deflating the score as coverage improves. - Divide by the new optional expectedMinimizerHits, falling back to nMinimizers when it is absent so pre-existing single-reference indices score identically - Deprecate nMinimizers while keeping it required for older clients - Validate the index on load, rejecting non-positive k, cutoff, or length and a non-finite or non-positive denominator so a malformed index fails loudly instead of yielding infinite or negative scores
- Add a typed minimizerIndex field (references, cutoff exponent) to VirusProperties so the generated pathogen.json schema documents the property instead of leaving it in the untyped catch-all - Without this, dataset builds validating against the schema flag minimizerIndex as an unknown property
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.
PR: multi-reference minimizer scoring for dataset suggestion
feat/multiref->masterPaired with
nextstrain/nextclade_databranchfeat/multiref(PR #404), which produces the index this change consumes. Test with CVA16 dataset using branchcva16-testing-sampleinnextclade_data.Related
minimizerReferencesrefactoring (superseded)What this enables
A single dataset can now be built from several reference sequences and still be suggested correctly by
nextclade sort. This matters for genetically diverse pathogens (for example enteroviruses), where one reference is not close enough to every circulating sequence for reliable auto-detection, and where splitting the pathogen into several datasets only creates ambiguity about which one to analyze a sequence with.The problem
Dataset suggestion sketches a query into a set of minimizers and, for each candidate dataset, counts how many of the query's minimizers appear in that dataset's stored set. The count is turned into a score by dividing by the dataset's stored minimizer count,
nMinimizers.When a dataset is built from several references, the index stores the union of the references' minimizers. A query only ever resembles one of those references, so its hit count stays roughly constant while
nMinimizersgrows with every reference added. Dividing by the union count therefore drives the score down as a dataset is given better coverage, which is exactly backwards: the datasets best able to recognize a sequence are penalized for it.The fix
Divide by the expected number of hits from a single reference instead of by the union size. For a reference of length$L$ , k-mer length $k$ , and acceptance cutoff $c$ over a $b$ -bit hash space, the expected count is
which is the number of scanned k-mers times the fraction retained by the cutoff.$E$ is invariant to how many references the dataset merges, so a query that matches one reference scores the same whether the dataset carries one reference or ten. Dividing by the union size would instead penalize a dataset purely for describing more of its own diversity.
The index now stores$E$ as a new optional
expectedMinimizerHitsfield, computed per reference at index-build time. A separate field is preferred over repurposingnMinimizersbecause:nMinimizersis an integer -- rounding the fractionalnMinimizersas the denominator; a new field lets them keep working unchangedFor single-reference datasets$E \approx$
nMinimizers, so the fallback to the integer field is safe and keeps older indexes scoring correctly.nMinimizersis deprecated but kept required for older clients.The index is also validated when loaded: a non-positive
k,cutoff, or reference length, and a non-finite or non-positive denominator, are rejected with an error rather than silently producing infinite or negative scores.What changes in each repository
expectedMinimizerHitsas the score denominator with anMinimizersfallback, deprecatenMinimizers, validate the index on load, regenerate the index JSON schema, and recognize theminimizerIndexconfig in the pathogen.json schema so dataset builds no longer flag it as an unknown property.feat/multiref): authorminimizerIndex.references(union of several FASTA files per dataset), compute and storeexpectedMinimizerHitsas the denominator, and guard degenerate inputs during the build.Deviations from agreed plan
expectedMinimizerHitsfield (f64) to the Nextclade index schema becausenMinimizersisi64-- rounding the fractional analytic denominator loses precision that affects score ordering between close datasets. Older clients fall back tonMinimizers(same value, rounded).expectedMinimizerHitsreplacing union-countnMinimizers) and (2) seven additional CVA16 references in the test index (denominator drops ~2020 -> ~462 due to shorter per-reference expectation). The test does not isolate the multi-reference contribution alone.Backward compatibility
The two changes are backward compatible and can be reviewed independently, but they must reach production together: until the data side emits
expectedMinimizerHits, this consumer falls back tonMinimizers, which on a multi-reference index is the union count and therefore the deflated denominator. No runtime warning distinguishes a healthy single-reference index from a deflated multi-reference one, so the pairing is enforced by coordinating the merge, not by the code.Verification
The scoring and validation paths are covered by serde round-trip tests over both the old and new index formats and by rejection tests for each validation rule. Full unit suite and clippy pass.
CLI: end-to-end
nextclade sortcomparisonBefore/after comparison using the CVA16 dataset (Coxsackievirus A16, lineages B1a/B1b/B1c/C/D/E/F) -- the motivating use case for multi-reference scoring.
Setup: build before/after binaries and indexes
Four artifacts needed. Assumes
nextcladeandnextclade_datarepos are checked out side by side.nextclade-mastermasternMinimizers)nextclade-multireffeat/multirefexpectedMinimizerHits)minimizer_index_before.jsonmasterminimizer_index_after.jsoncva16-testing-sampleTest 1: CVA16 example sequences (36 full-length genomes, ~7300-7400 bp)
Commands
All 36 sequences correctly classified as
enpen/enterovirus/cva16in both runs. Scores improved across the board:Per-sequence scores (36 sequences, sorted by delta)
OP562190 scores 0.987 -- it is one of the lineage references in the multi-ref set.
Test 2: Regression check -- mixed-pathogen queries (177 sequences)
9 CVA16 fragments (120 bp), 28 HBV, 12 HIV, 128 SARS-CoV-2.
Commands
0 assignment changes across all 177 sequences. 0 hit-count differences for the 138 classified sequences. The 9 CVA16-named queries are 120 bp fragments -- too short for minimizer matching (k=17). The 28 HBV sequences have no dataset in the index.
Conclusions
expectedMinimizerHitscorrectly compensates for union size inflation -- verified by zero hit-count differences for non-CVA16 datasetsWork items
expectedMinimizerHits, falling back tonMinimizerswhen absentexpectedMinimizerHitsto the minimizer index schema (regenerated JSON and YAML)nMinimizerswhile keeping it required for older clientsk,cutoff, length, and non-finite or non-positive denominator)approxdev-dependency for float assertionsminimizerIndexfield (references) to the pathogen.json config so the generated schema documents it instead of leaving it in the untyped catch-all