You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Bumps comet_version to "2026.03 rev. 0"; .idx format stays v5.
variable_modNN fields 5/6 (term_distance, which_term) honored on every search path
#130 deprecated the fifth and sixth variable_modNN fields, so position-restricted mods — N-terminal pyroglutamate -17.026549 Q 0 1 0 2 0 0.0 is the common case — applied to every Q. v2026.02.2 honored them on plain FASTA searches only; FI/PI index searches never did. This branch restores the FASTA behavior and adds the restrictions to the index paths.
Plain FASTA: v2026.02.2's position logic restored in CountVarMods / SubtractVarMods / HasVariableMod / VariableModSearch / MergeVarMods, with the ^/$-aware terminal tests. Several 2.2 bugs fixed on the way: binary mods with protein-C / peptide-C rules were never applied; a c-term mod's protein-N rule mixed start/end/"start+3" tests; per-end counts could carry into longer ends; a distance-restricted terminal mod could misalign site assignment. An n-term mod under a peptide-C-terminus rule is now placed when the peptide is short enough (2.2 silently never placed it).
FI/PI:PermuteIndexPeptideMods() passes each mod's rule to ModificationsPermuter, which computes a position-class byte per modifiable position (part of the dedup key) and ANDs each restricted mod's bitmask with it. Pool residues and all consumers are unchanged. Peptide-terminus rules and -2 are exact; protein-terminus rules are exact for d = 0 and, for d > 0, admit only peptides at that protein terminus (warned — the index has no position-in-protein). Protein attribution for protein-terminus rules goes through the same per-occurrence context bits as ^/$.
.idx header: each VariableMod: slot now carries :term_distance:which_term; 5-field slots from earlier builds load as unrestricted (how they were built). Header values get the same range check as comet.params.
n 0 0 / c 0 1 are still rewritten to ^ / $ (now silently); invalid values (term_distance < −2, or a distance rule with which_term outside 0–3) are rejected with an error.
AScorePro:AScoreOptions gains an optional peptidoform filter; Comet installs it (per thread, when a restricted slot exists) so AScorePro never scores or relocalizes onto a position a rule forbids. On FASTA the check uses the true protein offset (ProteinEntryStruct gains iProteinLength); on FI/PI it uses the row's flanks.
Real data (Hela, human.small.fasta, phospho + pyro-Q/E, internal decoys): honoring the rule gives +10.8% (xcorr) / +3.2% (E-value) PSMs at 1% FDR vs the #130 build; every target pyro-Glu lands on residue 1 on FASTA, FI and PI alike. Known limitation (documented): internal decoys move each mod with its reversed residue on every path, as in v2026.02.2.
Copies of a peptide repeated within one protein tied on every dedup-sort key and arrived in thread order, so the stored representative (mass bits, flanks) varied between builds. The comparators now break ties on the flanks (compared as unsigned char, so the order is platform-independent). Also: DBICompareByMass compared masses with a tolerance, which is not a strict weak ordering (UB in std::sort); it now uses a quantized integer key. New default-suite test T56 covers determinism without the big-data fixture.
index_search_type
Kept as the way to auto-build a peptide index. An issue-132 comment read index_search_type = 1 with a PEFF database as an index search; the parameter only selects the index type to auto-build for a missing .idx. Presence now means intent: comet -p no longer writes the parameter and comet -q writes index_search_type = -1 (not set; an auto-build then defaults to a fragment ion index). Any explicit 0 or 1 that cannot be honored (FASTA/PEFF database, an existing .idx of the other type, or a -i/-j build of the other type) warns, naming the file's type and the -i/-j to rebuild; other values warn and use the default. RealtimeSearch and tests/rts_repro likewise default to -1 and only send the parameter when one is given.
Note for existing params files: every file written by v2026.02.2's comet -p contains index_search_type = 1, so a plain FASTA search with such a file prints one warning line until that line is deleted. Documented in the release notes and on the website.
Tests and tooling
New: T54 (pyro-Glu on FASTA/FI/PI, .idx header, partial-rule warning, n-term/which_term 3), T55 (AScorePro filter), T56 (build determinism), T57 (position-rule edge cases: invalid values, binary rules, c-term protein-N rule, binary n-term group), T58 (FI/PI protein attribution), T59 (AScorePro FASTA protein offset), T60 (index_search_type scope); C++ PermuterTest P14–P18. T41/T49 rewritten for the restored semantics.
tests/regression/run_regression.py: default baseline fixed to v2026.02.2 (the old default was never installed, so the suite exited immediately); FI now runs with internal decoys, labeled "target-side only" against a pre-2026.03.0 baseline.
Design note: docs/20260930_varmod_position_restrictions.md; docs/20260915_permuter_terminal_mods.md D4 marked superseded; tests/tests.md updated (68 named tests + 21 legacy cases, 77 C++ cases).
Full run at 5155343: clean Windows and Linux builds; CometUnitTests 77/77 and the Python unit suite 79/79 on both; T17, T18, T22 ×4, T23, T24, T24b, T44 on both binaries (big-data parity and v2026.02.2 cross-version checks); regression suite 9/9 combinations with 1524/1524 top-peptide agreement vs v2026.02.2; .raw vs .mzXML on all 5 output formats.
The three commits after it (8966180, 6a8ddbf, 85893ca: index_search_type output and warnings, review round 4) touch Comet.cpp, CometSearchManager.cpp, a CometSearch.cpp flag, RTS/rts_repro defaults and tests; verified with the Python unit suite 79/79 on both binaries and CometUnitTests 77/77. Four code-review rounds; remaining findings are refactors, deferred and documented in docs/20260930_varmod_position_restrictions.md.
GeneratePlainPeptideIndex() sorts each length's peptide occurrences and takes
the first of every dedup run as the representative row, whose dPepMass,
flanks and original I/L letters are stored. Both comparators (long and
<=12-residue paths) ended at lProteinFileOffset, so copies of a peptide
repeated within one protein (e.g. UBC's ubiquitin repeats) tied on every key
and arrived in thread-scheduling order; std::sort then picked a different
copy from build to build. Those copies' masses differ in the last bits
(summed from different start positions), so two builds of human.small.fasta
differed in 4 bytes (MQIFVKTL... masses, ~1e-12 Da) and T18 failed --
a regression from v2026.02.2, which builds byte-identical indexes.
Break ties on every field the representative contributes (cPrevAA,
cNextAA, dPepMass, then the original sequence / uILMask), making the order
total; any remaining ties are identical in everything the merge reads.
Verified: T18 passes twice in a row (4 builds); 1-thread and 16-thread
builds are byte-identical; unit suite 72/72 including the byte-exact
reference .idx comparisons.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The variable_modNN fifth/sixth fields (term_distance, which_term) restrict
where a mod may go -- e.g. N-terminal pyroglutamate "-17.026549 Q 0 1 0 2".
v2026.02.2 applied them only on the plain-FASTA path; #130 deprecated them
and ignored them everywhere, so that config modified every Q.
Plain FASTA: restore v2026.02.2's position logic in CountVarMods(),
SubtractVarMods(), HasVariableMod(), VariableModSearch() and MergeVarMods(),
with the '^'/'$'-aware terminal tests. New VarModNtermCounted()/
VarModCtermCounted() are used both where VariableModSearch() counts terminal
sites and where MergeVarMods() consumes them, fixing a 2.2 misalignment for
distance-restricted n/c-term mods.
Params: fields 5/6 are no longer deprecated. 'n 0 0' / 'c 0 1' are still
rewritten to '^' / '$' (silently); the slot-merge step again compares the
two fields; the comet -p template documents them again.
FI/PI: CometFragmentIndex::PermuteIndexPeptideMods() passes each mod's rule
(ModPositionRule) to the permuter. getModifiableSequences() computes a
position-class byte per modifiable-sequence position (getPositionClass()),
made part of the dedup key and kept in a build-time parallel pool;
generateModifications() ANDs each restricted mod's bitmask with its class
bits. Pool residues and all consumers are unchanged. Peptide-terminus rules
and -2 are exact; protein-terminus d = 0 is exact; protein-terminus d > 0
admits only peptides at that protein terminus, with a warning.
.idx stays v5: each VariableMod: slot now also stores term_distance:
which_term; 5-field slots of earlier v5 files load as unrestricted.
Tests: T41 (renamed t41_termmod_fields) and T49 rewritten for the restored
semantics; T54 (pyroglutamate on FASTA/FI/PI, .idx header, partial-rule
warning); C++ PermuterTest P14-P17.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n residue
AScorePro knows a mod only as residues + mass, so with a restricted slot
(e.g. N-terminal pyroglutamate "Q 0 1 0 2") it scored, and Comet could adopt,
peptidoforms the search itself cannot produce, such as pyroglutamate on an
internal Q.
AScoreOptions gains an optional std::function<bool(const Peptide&)> filter
(copied by the copy constructor and assignment); AScoreCalculator skips a
generated peptidoform the filter rejects before scoring it, so it is neither
the top peptide nor a site-scoring alternative. CalculateAScorePro() sets the
filter, on a per-call copy of the shared g_AScoreOptions, only when a slot
AScorePro sees is restricted: a mod may move only to positions its rule
admits (ModificationsPermuter::getPositionClass()), or stay where Comet
placed it. The reported MOB and site scores are therefore computed among
allowed peptidoforms; when the original is the only one it keeps its own MOB
score and a 5000.0 site score.
Test: T55 (spectrum with pyroglutamate on the forbidden internal Q5 of
QTAGQPELK; FASTA/FI/PI with print_ascorepro_score = 1 keep Q1).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- comet_version "2026.03 rev. 0" (the withdrawn v2026.02.3 content plus this
branch ships as a new major release). Params files from 2026.0x are still
accepted.
- docs/20260930_varmod_position_restrictions.md: semantics, plain-FASTA
restoration, FI/PI position classes and support table, .idx v5 slot
format, AScorePro filter, the T18 determinism root cause, tests, known
limitations, and the Makefile header-dependency rebuild note.
- docs/20260915_permuter_terminal_mods.md: decision D4 marked superseded.
- tests/tests.md: T41 (t41_termmod_fields), T49 updated; T54, T55 and
PermuterTest P14-P17 added; counts (84 Python IDs, 76 C++ cases).
- tests/regression/run_regression.py: default baseline v2026.01.1 ->
v2026.02.2, matching setup_baselines.py (the old default is not
installed, so the suite exited immediately).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… only
run_regression.py skipped internaldecoy1/internaldecoy2 in fi mode because
FI had no internal decoys; it has them since 2026.03.0, so all three decoy
variants now run in fasta, fi and pi (the same small Hela test).
A baseline older than v2026.03.0 records DecoySearch in its FI index header
but generates no decoys (0 decoy rows, empty .decoy.txt), so those rows
compare a decoy-searching current build with a target-only baseline. The
report now marks them "NOTE: target-side only -- baseline <tag> predates FI
internal decoys (v2026.03.0)" (and flags the decoy_search=2 decoy-file block),
and the JSON metrics carry target_side_only: true. FI decoy generation itself
is covered by T34/T40/T22 (a spectrum matchable only by an internal decoy)
and T24b (full-scale parity).
Against v2026.02.2: fi/internaldecoy1 and fi/internaldecoy2 both 1524/1524
top-peptide agreement, 1524 PSMs >= 2.5 each.
tests/tests.md: variant table lists fasta, fi, pi for every variant, with
the target-side-only caveat.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Code-review follow-ups on the .idx build (GeneratePlainPeptideIndex()):
- DBICompareByMass() treated masses within FLOAT_ZERO (1e-6) as equal and then
ordered by sequence. That tolerance test is not transitive (a~b, b~c, a<c),
so std::sort got a comparator that is not a strict weak ordering (undefined
behavior; order could depend on input layout). Masses are now compared as
integer keys quantized to FLOAT_ZERO before the sequence tie-break. The
committed reference .idx fixtures are unchanged.
- The dedup-sort tie-breaks added for T18 after the flanks (dPepMass, sequence,
uILMask) were unreachable: copies of one sequence from one protein differ in
protein-terminus context, hence in cPrevAA or cNextAA. Dropped, with a
comment that says why the flanks complete the order.
- One reused scratch vector per length job for the per-run protein merge
instead of a heap allocation per unique peptide.
- Comparators compute splitClass() once per operand; the long path compares
sequences with memcmp when equal_I_and_L is off (bCanonEqual shares the
helper). Output byte-identical; no measurable build-time change at T18 scale.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
T18 checks build determinism at scale but needs human.small.fasta and runs
only with --integration. T56 generates a 400-protein FASTA in which every
protein repeats a 10-mer (short path) and a 14-mer (long path) at its
N-terminus and internally with a different next residue, so one protein
feeds the dedup merge several tuples of one sequence. No-enzyme builds
(length 8-15) with num_threads 1 and 16 must be byte-identical, with
equal_I_and_L 0 and 1. Fails on the pre-fix build (master), passes now.
tests/tests.md: T56 row; counts 64 named tests, 85 IDs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, tidy permuter/AScorePro
Code-review follow-ups on the variable_mod position-restriction work:
- Reject fifth/sixth-field values no search path defines (term_distance
below -2, or a distance rule with which_term outside 0-3) with an error;
they used to give different results on FASTA, FI/PI and AScorePro.
- Binary mods (v2026.02.2 code restored verbatim): a protein-C-terminus rule
counted sites against the peptide start instead of the residue, and a
peptide-C-terminus rule was never counted (the deferred per-end pass only
raised iTotVarModCt), so neither applied. Both now count like regular mods.
- c-term mod with a protein-N-terminus rule: the C-terminus position is
tested (iEndPos <= d) on every path (2.2 mixed start, end and "start + 3"
tests); HasVariableMod() and the pre-count are true upper bounds over the
candidate ends, so a start whose shorter ends qualify is no longer dropped
(also for a c-term mod with a peptide-N-terminus rule).
- Per-end counts are restored whenever they were snapshotted; when a later
check cleared bValid they used to carry into longer ends as extra
permutations.
- The deferred peptide-C-terminus pass runs only when a slot has such a rule.
- Permuter: a rule's class bit is forced to 1 where its mod cannot go, so such
positions no longer split the modifiable-sequence dedup key.
- AScorePro: whether any slot is restricted is decided once in
SetAScoreOptions() (g_bAScoreRestrictedSlots); each thread keeps a copy of
the options with the filter installed, refreshed on g_uiAScoreOptionsGeneration,
and only the filter's per-PSM context is updated per call.
Tests: T57 (invalid values rejected; binary peptide-C and protein-C rules;
c-term protein-N rule) -- all five checks fail on v2026.02.2; PermuterTest
P18 (fails without the class-bit change). Hela pyroglutamate and phospho
searches give byte-identical output before and after. tests/tests.md and the
design note updated.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… at that terminus
On the index paths a peptide shared by several proteins is one row whose
flanks are OR'd across its occurrences, so the permuter admits a residue mod
with a protein-terminus rule (variable_modNN fifth field >= 0, sixth 0/1,
e.g. "M 0 3 0 0") whenever the peptide is terminal in ANY protein -- and the
PSM was then reported against every protein containing it, unlike plain
FASTA, which evaluates each protein separately. '^'/'$' mods already narrow
attribution through the per-occurrence PROT_*_HERE context bits; these rules
now do the same.
- CometMassSpecUtils::ProteinTermRuleMask(slot): the terminus a slot's
protein-terminus rule requires.
- Output: ProteinTermContextMask() adds it for every placed mod, residue or
terminal (txt/pepXML protein lists, mzIdentML, RTS result path).
- Build time: CometPeptideIndex::ProteinTerminusContextMask() /
PassesProteinTerminusContext() walk the entry's residue slots too (new
iModSeqLen parameter); the FI and PI enumerations run the check when
g_bProteinTermRuleMods is set (computed in PermuteIndexPeptideMods()), not
only when terminal mods are active.
Test: T58 (MAGSPELK protein-N-terminal in one protein, internal in another;
the M[ox] PSM lists only the first on FASTA, FI_DB and PI_DB; the unmodified
control lists both). Fails on the previous build for FI_DB and PI_DB.
Unit suite 77/77 and CometUnitTests 77/77 on Linux and Windows.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The flank comparisons that complete the dedup sort's total order compared
plain char, whose signedness is platform-dependent: a flank byte >= 0x80
(stray UTF-8 in a FASTA) would rank lowest on x86/MSVC and highest on
aarch64 Linux, so the two builds could pick different representative copies
and the .idx would not be byte-identical across platforms. Compare as
unsigned char, matching memcmp's order used by the sequence key.
Unit suite 77/77 on Linux and Windows; T18 passes.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…eader validation, tighter bounds
Second code-review round on the variable_mod position-restriction work:
- An n-term mod under a peptide-C-terminus rule ('n ... d 3'), which
v2026.02.2 never placed (silent no-op), is admitted when the peptide's
C-terminus is within d of the N-terminus, mirroring the c-term /
which_term 2 case: VarModNtermCounted() takes iEndPos, the site is
counted by VariableModSearch()'s per-end pass, and getPositionClass()
admits the N-terminal sentinel when L-1 <= d.
- AScorePro filter on the plain-FASTA path: a protein-terminus rule is
evaluated on the true protein offset, not the flanks. ProteinEntryStruct
gains iProteinLength next to iStartResidue (set at every construction
site; 0 on the index paths); a placement is legal if any matched protein
admits it. The flank test would reject legal alternative sites and report
a 5000.0 "only possible site" score.
- VariableMod: slots read from a .idx header get the same term_distance /
which_term range check as comet.params instead of bypassing it.
- HasVariableMod() and the pre-count bound the c-term / which_term 2,
c-term / which_term 0 and n-term / which_term 3 cases by the shortest
storable end (peptide_length_range min) so a slot that can never place
its mod no longer drives the full enumeration for every start.
- Per-end count snapshot taken unconditionally before the enzyme check and
restored unconditionally (drops bSnapshotTaken).
- Permuter: the per-residue "irrelevant" mask is a 256-byte table computed
once per getModifiableSequences() call instead of string::find per
position.
Tests: T54 gains the n-term / which_term 3 case on all three paths; T59
(AScorePro FASTA protein offset) fails on the previous build; P17 updated.
Unit suite 78/78 and CometUnitTests 77/77 on Linux and Windows; Hela
pyroglutamate/phospho searches (AScorePro on and off) byte-identical.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The parameter has one effect -- which index type to auto-build when
database_name names an .idx that does not exist yet -- but it reads like a
search-mode switch (issue-132 comment: index_search_type = 1 with a PEFF
database was taken for an index search; it was a plain PEFF search, hence the
PEFF phospho mods). Keep the parameter (it is the way to auto-build a PI
index) and make its scope explicit:
- Warn at startup when it is set but has no effect: the database is not an
.idx (plain FASTA/PEFF search), or the .idx exists and its own
IndexSearchType: header line says the other type (the warning names the
file's type and the -i/-j to rebuild). An explicit -i/-j build stays quiet.
- Accept only 0/1; anything else warns and uses 1.
- comet -p one-liner now says "auto-build only ...; ignored for a FASTA or an
existing .idx"; RealtimeSearch usage text and comments updated (its arg 6
is the same auto-build selector; the stale "the file no longer implies a
mode" comment is gone).
Test: T60 (FASTA database warns and results are identical to the run without
the parameter; invalid value; missing .idx auto-built as PI/FI/FI for 0/1/
absent; existing FI .idx + 0 warns naming the type; -i build quiet). Unit
suite 79/79 and CometUnitTests 77/77 on Linux and Windows.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ount; cleanups
Third review round:
- The "ignored" warnings fired on the default: comet -p writes
index_search_type = 1 unconditionally and RealtimeSearch always sent 1, so
every default-params FASTA search and every RTS run against a peptide
index warned. 1 is the default everywhere and carries no intent; only an
explicit 0 that cannot be honored warns now (FASTA/PEFF database, or an
existing fragment ion index). RealtimeSearch parses a missing arg 6 as -1
and sends the parameter only when it was given.
- Binary groups: an n-term mod under a which_term 3 rule with a plain n-term
mate counted its one physical site twice (start-residue pass via the mate,
then the per-end pass), failing the group-sum check and dropping the
peptide. The per-end pass skips the binary increment when the start pass's
mate fallback applied.
- Cleanups left by the unconditional snapshot: the "suppress gcc warning"
pre-initialization and the bare block around the restore loop.
- Documented: the AScorePro FASTA protein-offset check sees only the proteins
recorded for the PSM (capped by max_duplicate_proteins).
Tests: T60 asserts the default 1 is quiet on a FASTA database and against a
peptide index (-j build) while 0 warns; T57 adds the binary-group case. Unit
suite 79/79 and CometUnitTests 77/77 on Linux and Windows; RealtimeSearch
rebuilt.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…-q, symmetric warnings
Presence now means intent. comet -p writes only the explanatory comment
block; comet -q writes "index_search_type = -1" (-1 is the internal "not
set" sentinel: fragment ion index for an auto-build, never a warning). The
validator accepts -1/0/1 and falls back to the default (-1) for anything
else, with a warning.
With no default-generated 0/1 in circulation the warnings are symmetric: any
explicit 0 or 1 is ignored with a warning when the database is a FASTA/PEFF
(plain search), or when an existing .idx records the other type (the
warning names the file's type and the -i/-j to rebuild); a matching value,
-1 or an absent parameter is quiet, and so are explicit -i/-j builds.
T60 checks the -p/-q template lines and the -1 cases. Unit suite 79/79 and
CometUnitTests 77/77 on Linux and Windows.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The explanatory comment block moves under the -q branch of the comet -p/-q
template, next to the "index_search_type = -1" line it describes, so a -p
params file does not mention the parameter at all (like the PEFF and
fragindex entries). No logic change. T60 asserts -p has no mention and -q
has the -1 line. Unit suite 79/79 on Linux.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…count flag, T57 control
- An explicit index_search_type that disagrees with a -i/-j build now warns
at build time ("index_search_type = 0 is overridden by -i: building a
fragment ion index") instead of only at the first search of the result.
- RealtimeSearch: an invalid 6th argument is ignored (not sent, native
default) rather than sent as 1; message reworded. tests/rts_repro mirrors
RealtimeSearch (default -1, parameter sent only when given).
- The per-end which_term 3 pass reads a per-slot flag (pbNtermViaMate) that
the start-residue binary pass sets when it counts a group's n-term site
through a mate, instead of re-deriving that pass's fallback predicate.
- T57: positive control for the binary-group 10-mer (takes the unrestricted
mate), so the negative check cannot pass vacuously. T60: -i/-j override
warnings, agreeing build quiet. tests/tests.md wording for T57 fixed.
Unit suite 79/79 and CometUnitTests 77/77 on Linux and Windows; T22
(rts_repro) passes.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Restores position-restricted variable modifications across FASTA, indexed, and AScorePro paths while improving index determinism and index_search_type handling.
…filter, design note
- Binary mod under -2 (not on the peptide C-terminal residue): the cumulative residue
pass counted the C-terminal residue as a site, so the group's all-or-nothing count
never matched the sites MergeVarMods() can fill and the only valid peptidoform was
never generated (also in v2026.02.2). The per-end pass now takes that one site back
out of the group total after the snapshot. T57: QTAGSPELK[+8.01]AGSPELK under
'K 1 3 -2 0'.
- Regression suite: "target-side only" rows (FI + internal decoys vs a pre-2026.03.0
baseline) now search both binaries with num_output_lines = 5 and drop decoy_prefix
PSMs before the comparison, so a concatenated decoy that outscores the target cannot
displace it; the label was previously report-only.
- Design note: drop the obsolete "which_term 3 never admits an n-term mod" condition;
record the binary -2 fix.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Filter configurations before truncating to 1,000 candidates
AScorePro/AScoreCalculator.cpp:145
Filtering here occurs after UnifiedPeptideGenerator::initTargetMod() has already truncated its configurations to 1,000. With two restricted copies on a 50-residue peptide, the only legal placement can occur after the first 1,000 of 1,225 combinations, so it is discarded before this filter runs and AScore returns no legal candidate despite the original placement being valid. Apply the filter during generation/before truncation, then cap the accepted candidates.
…ule; target-side regression rows use separate decoys
- VariableModSearch(): a binary group's site is claimed by the first member, in slot
order, whose residues / terminal code match it and whose OWN distance rule admits it
(BinarySiteClaimed()). 2.2 and the restored code judged a group mate's residues under
the first slot's rule, so an unrestricted first slot admitted a restricted mate at
forbidden positions and a restricted first slot dropped an unrestricted mate's sites.
Members whose rule needs the peptide end (-2, which_term 3) are deferred to the
per-end pass by the same helper, replacing the -2 decrement block and pbNtermViaMate.
T57: 'K 1 3 -1 0' + 'S 1 3 0 0' and the reversed order both generate AGSPELK[+8.01].
- Regression suite: for "target-side only" rows the current binary builds and searches
with decoy_search = 2, so its decoys go to a separate file and the compared file holds
every spectrum's best target at any rank depth; the decoy-prefix filter stays as a
guard. A fixed rank depth could drop a spectrum outranked by several decoys.
- Design note and tests.md updated.
Co-Authored-By: Claude Fable 5.1 <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
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.
Bumps
comet_versionto "2026.03 rev. 0";.idxformat stays v5.variable_modNN fields 5/6 (term_distance, which_term) honored on every search path
#130 deprecated the fifth and sixth
variable_modNNfields, so position-restricted mods — N-terminal pyroglutamate-17.026549 Q 0 1 0 2 0 0.0is the common case — applied to every Q. v2026.02.2 honored them on plain FASTA searches only; FI/PI index searches never did. This branch restores the FASTA behavior and adds the restrictions to the index paths.CountVarMods/SubtractVarMods/HasVariableMod/VariableModSearch/MergeVarMods, with the^/$-aware terminal tests. Several 2.2 bugs fixed on the way: binary mods with protein-C / peptide-C rules were never applied; a c-term mod's protein-N rule mixed start/end/"start+3" tests; per-end counts could carry into longer ends; a distance-restricted terminal mod could misalign site assignment. An n-term mod under a peptide-C-terminus rule is now placed when the peptide is short enough (2.2 silently never placed it).PermuteIndexPeptideMods()passes each mod's rule toModificationsPermuter, which computes a position-class byte per modifiable position (part of the dedup key) and ANDs each restricted mod's bitmask with it. Pool residues and all consumers are unchanged. Peptide-terminus rules and-2are exact; protein-terminus rules are exact for d = 0 and, for d > 0, admit only peptides at that protein terminus (warned — the index has no position-in-protein). Protein attribution for protein-terminus rules goes through the same per-occurrence context bits as^/$..idxheader: eachVariableMod:slot now carries:term_distance:which_term; 5-field slots from earlier builds load as unrestricted (how they were built). Header values get the same range check ascomet.params.n 0 0/c 0 1are still rewritten to^/$(now silently); invalid values (term_distance < −2, or a distance rule with which_term outside 0–3) are rejected with an error.AScoreOptionsgains an optional peptidoform filter; Comet installs it (per thread, when a restricted slot exists) so AScorePro never scores or relocalizes onto a position a rule forbids. On FASTA the check uses the true protein offset (ProteinEntryStructgainsiProteinLength); on FI/PI it uses the row's flanks.Real data (Hela,
human.small.fasta, phospho + pyro-Q/E, internal decoys): honoring the rule gives +10.8% (xcorr) / +3.2% (E-value) PSMs at 1% FDR vs the #130 build; every target pyro-Glu lands on residue 1 on FASTA, FI and PI alike. Known limitation (documented): internal decoys move each mod with its reversed residue on every path, as in v2026.02.2..idx build determinism (T18 regression from #130)
Copies of a peptide repeated within one protein tied on every dedup-sort key and arrived in thread order, so the stored representative (mass bits, flanks) varied between builds. The comparators now break ties on the flanks (compared as
unsigned char, so the order is platform-independent). Also:DBICompareByMasscompared masses with a tolerance, which is not a strict weak ordering (UB instd::sort); it now uses a quantized integer key. New default-suite test T56 covers determinism without the big-data fixture.index_search_type
Kept as the way to auto-build a peptide index. An issue-132 comment read
index_search_type = 1with a PEFF database as an index search; the parameter only selects the index type to auto-build for a missing.idx. Presence now means intent:comet -pno longer writes the parameter andcomet -qwritesindex_search_type = -1(not set; an auto-build then defaults to a fragment ion index). Any explicit0or1that cannot be honored (FASTA/PEFF database, an existing.idxof the other type, or a-i/-jbuild of the other type) warns, naming the file's type and the-i/-jto rebuild; other values warn and use the default.RealtimeSearchandtests/rts_reprolikewise default to -1 and only send the parameter when one is given.Note for existing params files: every file written by v2026.02.2's
comet -pcontainsindex_search_type = 1, so a plain FASTA search with such a file prints one warning line until that line is deleted. Documented in the release notes and on the website.Tests and tooling
.idxheader, partial-rule warning, n-term/which_term 3), T55 (AScorePro filter), T56 (build determinism), T57 (position-rule edge cases: invalid values, binary rules, c-term protein-N rule, binary n-term group), T58 (FI/PI protein attribution), T59 (AScorePro FASTA protein offset), T60 (index_search_typescope); C++PermuterTestP14–P18. T41/T49 rewritten for the restored semantics.tests/regression/run_regression.py: default baseline fixed to v2026.02.2 (the old default was never installed, so the suite exited immediately); FI now runs with internal decoys, labeled "target-side only" against a pre-2026.03.0 baseline.docs/20260930_varmod_position_restrictions.md;docs/20260915_permuter_terminal_mods.mdD4 marked superseded;tests/tests.mdupdated (68 named tests + 21 legacy cases, 77 C++ cases).Validation (final commit 5155343)
Full run at 5155343: clean Windows and Linux builds;
CometUnitTests77/77 and the Python unit suite 79/79 on both; T17, T18, T22 ×4, T23, T24, T24b, T44 on both binaries (big-data parity and v2026.02.2 cross-version checks); regression suite 9/9 combinations with 1524/1524 top-peptide agreement vs v2026.02.2;.rawvs.mzXMLon all 5 output formats.The three commits after it (8966180, 6a8ddbf, 85893ca:
index_search_typeoutput and warnings, review round 4) touchComet.cpp,CometSearchManager.cpp, aCometSearch.cppflag, RTS/rts_repro defaults and tests; verified with the Python unit suite 79/79 on both binaries andCometUnitTests77/77. Four code-review rounds; remaining findings are refactors, deferred and documented indocs/20260930_varmod_position_restrictions.md.