Stop publishing two more sentinels as measurements - #794
Merged
Conversation
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>
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.
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