Skip to content

Stop publishing two more sentinels as measurements - #794

Merged
htsugawa merged 1 commit into
masterfrom
fix/uncomputed-similarity-terms
Sep 6, 2026
Merged

Stop publishing two more sentinels as measurements#794
htsugawa merged 1 commit into
masterfrom
fix/uncomputed-similarity-terms

Conversation

@htsugawa

@htsugawa htsugawa commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

The candidate export shipped with a comment asserting that its five non-spectral
similarity terms have no equivalent of the -1 sentinel, so an unused term is
stored as 0 and cannot be told apart from a term evaluated as 0. That is wrong
for every one of them, and it was wrong in two different ways.

GetIsotopeRatioSimilarity returns -1 when there is nothing to compare -- either
side carries no isotopic peaks, or a monoisotopic abundance is not positive.
Unlike the dot products there is no clamping getter, so the -1 reaches the export
unchanged. On the public negative reference library used for the reanalysis
pilot, 495 of 4972 exported candidate rows carried exactly -1, every one of them
an in-house record with no isotopic pattern deposited, and the companion catalog
stored all 495 as a measurement.

GetGaussianSimilarity, which produces the retention-time, retention-index,
collision-cross-section and accurate-mass terms, also returns -1 when either
value is missing. It is otherwise exp(-0.5 * ((actual - reference) / tolerance)^2),
strictly positive until it underflows past roughly 38 tolerance widths. So a term
the run never enabled is left at the field's default 0, which is what all 4972
rows of that same export carried for retention time, retention index and CCS --
those terms were disabled for the run.

The two need opposite rules, and the reason is the shape of the function rather
than a convention. The Gaussian terms are positive exactly when they were
measured, so only a positive value is published; 0 is either the unset default or
an underflow and neither is worth publishing, which is also the test GetTotalScore
already applies before adding a term to the total. The isotope term is 1 minus an
accumulated ratio difference and is genuinely signed, so a negative value there is
a measurement, and a strong one: it says the patterns disagree. Only exactly -1 is
its sentinel.

Both rules live on MsScanMatchResult, beside IsSpectrumComparisonPerformed, so the
scoring side and the export side cannot drift apart. The remaining ambiguity is
stated rather than hidden: a computed isotope similarity of exactly -1 is
indistinguishable from an unattempted comparison, and is resolved as not computed,
because publishing a sentinel as a measurement is the worse of the two errors.

Three tests, one per rule and one for the case that would be lost by applying the
Gaussian rule to the isotope term. 328 tests pass.

Follows #793, which introduced the comment this corrects.

🤖 Generated with Claude Code

The candidate export shipped with a comment asserting that its five non-spectral
similarity terms have no equivalent of the -1 sentinel, so an unused term is
stored as 0 and cannot be told apart from a term evaluated as 0. That is wrong
for every one of them, and it was wrong in two different ways.

GetIsotopeRatioSimilarity returns -1 when there is nothing to compare -- either
side carries no isotopic peaks, or a monoisotopic abundance is not positive.
Unlike the dot products there is no clamping getter, so the -1 reaches the export
unchanged. On the public negative reference library used for the reanalysis
pilot, 495 of 4972 exported candidate rows carried exactly -1, every one of them
an in-house record with no isotopic pattern deposited, and the companion catalog
stored all 495 as a measurement.

GetGaussianSimilarity, which produces the retention-time, retention-index,
collision-cross-section and accurate-mass terms, also returns -1 when either
value is missing. It is otherwise exp(-0.5 * ((actual - reference) / tolerance)^2),
strictly positive until it underflows past roughly 38 tolerance widths. So a term
the run never enabled is left at the field's default 0, which is what all 4972
rows of that same export carried for retention time, retention index and CCS --
those terms were disabled for the run.

The two need opposite rules, and the reason is the shape of the function rather
than a convention. The Gaussian terms are positive exactly when they were
measured, so only a positive value is published; 0 is either the unset default or
an underflow and neither is worth publishing, which is also the test GetTotalScore
already applies before adding a term to the total. The isotope term is 1 minus an
accumulated ratio difference and is genuinely signed, so a negative value there is
a measurement, and a strong one: it says the patterns disagree. Only exactly -1 is
its sentinel.

Both rules live on MsScanMatchResult, beside IsSpectrumComparisonPerformed, so the
scoring side and the export side cannot drift apart. The remaining ambiguity is
stated rather than hidden: a computed isotope similarity of exactly -1 is
indistinguishable from an unattempted comparison, and is resolved as not computed,
because publishing a sentinel as a measurement is the worse of the two errors.

Three tests, one per rule and one for the case that would be lost by applying the
Gaussian rule to the isotope term. 328 tests pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@htsugawa
htsugawa merged commit e6f724e into master Sep 6, 2026
9 checks passed
@htsugawa
htsugawa deleted the fix/uncomputed-similarity-terms branch September 6, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant