Skip to content

Commit 034002b

Browse files
trishortsclaude
andcommitted
fix(truncation): gate diagnostic file; strengthen FDR/score tests
- TruncationSearchTask: write CandidateRanks.tsv only when PerfLogPath is set, register it via FinishedWritingFile, and wrap it best-effort so a diagnostic write can't abort the task (#14). Document the intentional PEP==0 disk-filter guard and the in-memory-only PepQWasUsed metric (#11, #15 -- wontfix). - TruncationSearchEngineTests: ScanS7 now asserts the opposing-series peaks neither raise the score nor change the integer matched-ion count, so the test distinguishes "ignored" from merely "harmless" (#17). - TruncationFdrAndOutputTests: PepRuns pins the skipped-PEP branch (PEP_QValue == 2 on a too-small pool) rather than only asserting no throw (#18). Addresses findings #14, #17, #18 and documents #11/#15 from the PR #2666 review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DWZ1KbGenS4w1beTMXWgek
1 parent 4ea0d06 commit 034002b

3 files changed

Lines changed: 38 additions & 3 deletions

File tree

MetaMorpheus/TaskLayer/TruncationSearchTask/TruncationSearchTask.cs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,23 @@ protected override MyTaskResults RunSpecific(string OutputFolder, List<DbForTask
230230
WriteOutputs(withFdr, OutputFolder, taskId);
231231

232232
// Diagnostic side file: per-scan winning-parent rank in the index match-count ordering.
233-
string ranksPath = Path.Combine(OutputFolder, "CandidateRanks.tsv");
234-
File.WriteAllLines(ranksPath,
235-
new[] { "File\tScanNumber\tCandidateRank\tCandidatePoolSize\tScore\tProducedTruncation" }.Concat(winnerRankRows));
233+
// Opt-in (gated on the same PerfLogPath that enables benchmarking), registered via
234+
// FinishedWritingFile, and best-effort so a diagnostic-write failure can't abort an otherwise
235+
// successful task (#14).
236+
if (!string.IsNullOrWhiteSpace(TruncationSearchParameters.PerfLogPath))
237+
{
238+
try
239+
{
240+
string ranksPath = Path.Combine(OutputFolder, "CandidateRanks.tsv");
241+
File.WriteAllLines(ranksPath,
242+
new[] { "File\tScanNumber\tCandidateRank\tCandidatePoolSize\tScore\tProducedTruncation" }.Concat(winnerRankRows));
243+
FinishedWritingFile(ranksPath, new List<string> { taskId });
244+
}
245+
catch (Exception ex)
246+
{
247+
Warn($"Could not write diagnostic CandidateRanks.tsv: {ex.Message}");
248+
}
249+
}
236250

237251
// 6. Optional perf-log row (03_Benchmarks). No-op unless a path is configured.
238252
if (!string.IsNullOrWhiteSpace(TruncationSearchParameters.PerfLogPath))
@@ -362,6 +376,8 @@ private List<TruncationParent> BuildParentsFromPsms(IEnumerable<SpectralMatch> p
362376
}
363377

364378
// Record whether the PEP q-value (vs notch q-value) drove inclusion (perf logging).
379+
// By design this reflects the in-memory parent path only; the disk/db-seeded paths leave the
380+
// metric false because PsmFromTsv's PEP column is ambiguous there (#15, intentional).
365381
FdrInfo fdr = psm.GetFdrInfo(peptideLevel: true) ?? psm.GetFdrInfo(peptideLevel: false);
366382
if (fdr != null && fdr.PEP_QValue != 2)
367383
{
@@ -573,6 +589,11 @@ private List<TruncationParent> BuildParentsFromDisk()
573589
/// <summary>Permissive parent filter (#3) for disk rows (PsmFromTsv stores PEP_QValue/QValueNotch).</summary>
574590
private static bool PassesDiskParentFilter(PsmFromTsv row, double threshold)
575591
{
592+
// 0 is treated as "PEP not computed" alongside the 2 sentinel ON PURPOSE for disk rows:
593+
// PsmFromTsv defaults an absent/empty PEP_QValue column to 0, so a disk 0 is ambiguous (absent
594+
// column vs a genuine best-possible 0.0) and the two cannot be told apart here. Falling back to
595+
// the notch q-value when PEP==0 is the safe choice. Do NOT "simplify" this to `!= 2` only -- that
596+
// would pass absent-PEP rows as if q=0 (reviewed: findings #11/#15, intentional wontfix).
576597
if (row.PEP_QValue != 0 && row.PEP_QValue != 2)
577598
{
578599
return row.PEP_QValue <= threshold;

MetaMorpheus/Test/TruncationFdrAndOutputTests.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ public void PepRuns_OrSkipsCleanly_OnSmallPool()
7979
new List<(string, CommonParameters)> { ("synthetic", _cp) }, "TruncationTask", doPep: true));
8080

8181
Assert.That(result.All(p => p.FdrInfo.QValue >= 0 && p.FdrInfo.QValue <= 1), Is.True);
82+
83+
// Pin the branch, not just absence-of-throw: a 2-PSM pool can't train PEP, so PEP must be
84+
// SKIPPED -- every row keeps the "PEP not computed" sentinel (PEP_QValue == 2), rather than
85+
// silently emitting an untrained PEP q-value (#18).
86+
Assert.That(result.All(p => p.FdrInfo.PEP_QValue == 2), Is.True);
8287
}
8388

8489
[Test]

MetaMorpheus/Test/TruncationSearchEngineTests.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,15 @@ public void ScanS7_OpposingIonsIgnored()
214214
Assert.That(s7.Outcome, Is.EqualTo(TruncationScanOutcome.Winner));
215215
Assert.That(s7.WinningParent.ProteinAccession, Is.EqualTo("P1"));
216216
Assert.That(s7.WinningSeries, Is.EqualTo(FragmentationTerminus.N));
217+
218+
// The opposing-series peaks must not be MATCHED: they can only dilute the intensity-normalized
219+
// score (they enlarge the total-intensity denominator), never raise it, and must not change the
220+
// integer matched-ion count -- i.e. the score moves by less than one ion. (Verified empirically:
221+
// exact equality fails because the two extra peaks shift the normalization by ~0.05; a matched
222+
// opposing ion would instead RAISE the score by ~1.) (#17)
223+
var s2 = _results[1];
224+
Assert.That(s7.Score, Is.LessThanOrEqualTo(s2.Score));
225+
Assert.That(s2.Score - s7.Score, Is.LessThan(1.0));
217226
}
218227

219228
[Test]

0 commit comments

Comments
 (0)