@@ -614,13 +667,15 @@
Per-file calibration corrections
hasStructural:true,
features:D.model||[],modelDegenerate:D.modelDegenerate,scores:D.scores,
densityRatio:D.densityRatio,winFraction:D.winFraction,
- fdpViews:D.fdpViews,idYield:D.idYield,crossRun:D.crossRun,perFile:D.perFile};
+ fdpViews:D.fdpViews,idYield:D.idYield,crossRun:D.crossRun,perFile:D.perFile,
+ coAssignment:D.coAssignment};
const m=D.pass2.model; // null under confidence-transfer mode
return {pass:2,label:"2nd pass",desc:"post-reconciliation reported pool",
hasStructural:!!m,
features:m?(m.features||[]):[],modelDegenerate:m?m.degenerate:false,scores:m?m.scores:null,
densityRatio:D.pass2.densityRatio,winFraction:D.pass2.winFraction,
- fdpViews:D.pass2.fdpViews,idYield:D.pass2.idYield,crossRun:D.pass2.crossRun,perFile:D.pass2.perFile};
+ fdpViews:D.pass2.fdpViews,idYield:D.pass2.idYield,crossRun:D.pass2.crossRun,perFile:D.pass2.perFile,
+ coAssignment:D.pass2.coAssignment};
}
const PASSES=[passBundle(1)];
if(D.pass2)PASSES.push(passBundle(2));
@@ -1545,6 +1600,10 @@
Per-file calibration corrections
// Structural card (built from the pass's reported-pool scores). Under Pass 2
// confidence-transfer (no retrained model) it degrades to an n/a note.
function renderCompetitionTab(b){
+ // Q-driven, so it renders on every pass INCLUDING pass-2 confidence transfer, where the
+ // structural coin above degrades to an n/a note. Called before those early returns for
+ // exactly that reason.
+ renderCoAssignment(b.coAssignment);
if(!b.hasStructural){
naChart("wfChart","wfLegend",PASS2_NA_COMPETITION);
document.getElementById("wfKpis").innerHTML="";document.getElementById("wfNote").innerHTML="";
@@ -1602,6 +1661,211 @@
Per-file calibration corrections
: "Without an entrapment library this is confounded by true positives, but a real coin well below 50% in the null band is still suspect.");
}
+// ========== Competition tab: single-peak multiple-ID co-assignment (#4522) ==========
+// Q-driven: renders on any pass that produced q-values. The card hides itself entirely when the
+// panel is absent (no apex RT source could be reconstructed), rather than showing zeros - a zero
+// co-assignment rate and "could not measure" are very different claims.
+// Scope order and labels match the FDR tab's selector exactly: experiment-wide FIRST, so the
+// broad view is the default everywhere and the per-run drill-down is always the secondary step.
+let caScope=0; // 0 = experiment-wide, 1 = per-run (same indexing as fdrScope)
+function renderCoAssignment(ca){
+ const card=document.getElementById("caCard");
+ if(!card)return;
+ if(!ca||!ca.run){card.style.display="none";return;}
+ card.style.display="";
+ const SCOPES=[["experiment","experiment-wide"],["run","per-run"]];
+ const sel=document.getElementById("caScope");sel.innerHTML="";
+ SCOPES.forEach((sc,k)=>{const btn=H(sel,"button",{class:"vbtn"+(k===caScope?" on":"")},sc[1]);
+ btn.onclick=()=>{if(caScope===k)return;caScope=k;renderCoAssignment(ca);};});
+ const s=ca[SCOPES[caScope][0]];
+ if(!s){document.getElementById("caTable").innerHTML=
+ "
No co-assignment data at this q scope.
";return;}
+
+ const rows=[["Target",s.target,COL.target],["Entrapment",s.entrapment,COL.ptarget],
+ ["Decoy",s.decoy,COL.decoy]].filter(r=>r[1]);
+ // ---- KPIs: the false-class rate, the base rate it must be read against, the ratio ----
+ const kp=document.getElementById("caKpis");kp.innerHTML="";
+ if(s.entrapment)kpi(kp,"entrapment on a peak a better target explains",pct(s.entrapment.betterFraction,1),"bad");
+ kpi(kp,"target base rate (same test)",pct(s.target.betterFraction,1),"");
+ if(s.entrapment)kpi(kp,"entrapment enrichment",isFinite(s.enrichment)?fmt(s.enrichment,1)+"×":"–",
+ isFinite(s.enrichment)&&s.enrichment>1.5?"bad":"");
+ kpi(kp,"target IDs with no distinguishing evidence",fmt(s.target.nBetter,0),"");
+
+ // ---- the composite acceptance score, and what this pass's own pool needs to reach the FDR ----
+ // Two numbers, because they answer different questions and their DIVERGENCE is the signal.
+ // The per-entry composite score does not change between passes, but the score at which the
+ // target-decoy competition reaches a given q is a property of the POPULATION counted - so a
+ // pass whose pool has been compacted needs a different score for the same q. They agree at
+ // pass 1; a large gap at pass 2 says the pool the decoys are drawn from has been selected.
+ // NOT a threshold to adopt: the decoy and non-decoy counts beside it move TOGETHER, so a
+ // lower crossing buys targets and entrapment at the same error rate it buys decoys.
+ const cut=document.getElementById("caCutoff");
+ if(cut){
+ cut.innerHTML="";
+ // Either number can be present without the other: the crossing is computed from the
+ // whole population while the cutoff needs an accepted precursor, so a run with none has a
+ // NaN cutoff and a finite crossing. Gating the table on the cutoff hid a value the golden
+ // pins and the caption below discusses.
+ if(isFinite(ca.experimentCutoff)||isFinite(ca.experimentFdrCrossing)){
+ const t=H(cut,"table"),h=H(H(t,"thead"),"tr");
+ ["Composite acceptance score","Score","Target + entrapment","Decoys"]
+ .forEach((x,i)=>H(h,"th",{class:i===0?"l":""},x));
+ const tb2=H(t,"tbody");
+ const r1=H(tb2,"tr");
+ H(r1,"td",{class:"l"},"worst accepted precursor (the bar in use)");
+ H(r1,"td",{class:"num"},isFinite(ca.experimentCutoff)?fmt(ca.experimentCutoff,4):"–");
+ H(r1,"td",{class:"num"},"–");H(r1,"td",{class:"num"},"–");
+ const r2=H(tb2,"tr");
+ H(r2,"td",{class:"l"},"where this pass's own pool reaches the target FDR");
+ H(r2,"td",{class:"num"},isFinite(ca.experimentFdrCrossing)?fmt(ca.experimentFdrCrossing,4):"–");
+ H(r2,"td",{class:"num"},fmt(ca.experimentFdrCrossingNonDecoys,0));
+ H(r2,"td",{class:"num"},fmt(ca.experimentFdrCrossingDecoys,0));
+ }
+ }
+
+ // ---- per-class table ----
+ const host=document.getElementById("caTable");host.innerHTML="";
+ const tb=H(host,"table"),th=H(H(tb,"thead"),"tr");
+ ["Class","Detected","Shares a peak","Outscored by a target","Rate","vs base rate","of which PTM isomer"]
+ .forEach((h,i)=>H(th,"th",{class:i===0?"l":""},h));
+ const body=H(tb,"tbody");
+ rows.forEach(r=>{const tr=H(body,"tr"),c=r[1];
+ const cell=H(tr,"td",{class:"l"});
+ H(cell,"span",{class:"swatch",style:"display:inline-block;width:9px;height:9px;border-radius:2px;margin-right:6px;background:"+r[2]});
+ cell.appendChild(document.createTextNode(r[0]));
+ [fmt(c.n,0),fmt(c.nShared,0),fmt(c.nBetter,0),pct(c.betterFraction,2)].forEach(v=>H(tr,"td",{class:"num"},v));
+ const ratio=s.target.betterFraction>0?c.betterFraction/s.target.betterFraction:NaN;
+ // Suppress on the TARGET denominator too, not just this row's n. CoAssignmentAccumulator.Build
+ // gates on both, so gating on one here prints a ratio the KPI beside it withholds - computed
+ // off a denominator too small to mean anything, which is the arithmetic-noise-as-finding that
+ // MIN_N_FOR_ENRICHMENT was serialized to the payload to prevent (one definition, not one per
+ // renderer).
+ const suppressed=c.n
"+fmt(ent.nBetter,0)+" of "+fmt(ent.n,0)+" entrapment ("+pct(ent.betterFraction,1)+")");
+ if(dec)parts.push(""+fmt(dec.nBetter,0)+" of "+fmt(dec.n,0)+" decoys ("+pct(dec.betterFraction,1)+")");
+ note+=(parts.length?parts.join(" and ")+" would disappear":"nothing measurable would disappear")+
+ " if a best-match-wins rule were enforced on peaks claimed by two entries. The same rule would "+
+ "drop "+fmt(s.target.nBetter,0)+" target IDs, which may well be real — that is its "+
+ "cost, and why this is a diagnostic and not a filter.";
+ if(s.target.nBetterSameBaseSequence>0)
+ note+=" "+fmt(s.target.nBetterSameBaseSequence,0)+" of those target pairs differ only in "+
+ "modification placement: isobaric PTM positional isomers genuinely co-elute at one m/z with "+
+ "BOTH present, so a winner-takes-all rule would delete one of a real pair. Those need a "+
+ "distinguishing transition to quantify separately, not a winner.";
+ if(ca.pass===2)
+ note+=" This is the reported pool (post-reconciliation). Compare it with the 1st pass: "+
+ "compaction removes co-assigned precursors, so the rate falls, while the entrapment "+
+ "enrichment typically RISES because the pool sheds co-assigned targets faster.";
+ document.getElementById("caNote").innerHTML=note;
+
+ drawCaHist(ca,s);
+ drawCaLadder(ca,s);
+ drawCaOffenders(s);
+}
+
+// |dRT| histogram over the FULL scanned window, each class normalized to its own total.
+// Not truncated at the headline tolerance: truncating would hide the very shape (a spike at
+// small offsets decaying into a flat chance floor) that justifies the tolerance.
+function drawCaHist(ca,s){
+ const host=document.getElementById("caHist");host.innerHTML="";
+ const ed=ca.deltaRtEdges;
+ if(!ed||ed.length<2){host.innerHTML="No co-assigned pairs to bin.
";return;}
+ const norm=a=>{if(!a)return null;const t=a.reduce((x,y)=>x+y,0);return t>0?a.map(v=>v/t):null;};
+ const tn=norm(s.deltaRtTarget), en=norm(s.deltaRtEntrapment);
+ if(!tn&&!en){host.innerHTML="No co-assigned pairs to bin.
";return;}
+ const W=520,Hh=250,box={l:52,t:14,w:W-70,h:Hh-58};
+ let mx=0;[tn,en].forEach(a=>{if(a)a.forEach(v=>{if(v>mx)mx=v;});});
+ const sv=svg(W,Hh);host.appendChild(sv);
+ const sc=axes(sv,box,[ed[0],ed[ed.length-1]],[0,mx*1.08],
+ {xlab:"|apex RT difference| (min)",ylab:"fraction of pairs",yfmt:v=>pct(v,0)});
+ const bw=(box.w/(ed.length-1));
+ function bars(a,color,inset,op){if(!a)return;a.forEach((v,i)=>{if(v<=0)return;
+ E(sv,"rect",{x:sc.x(ed[i])+bw*inset,y:sc.y(v),width:Math.max(1,bw*(1-2*inset)),
+ height:box.t+box.h-sc.y(v),fill:color,opacity:op});});}
+ bars(tn,COL.target,0.02,.65);
+ bars(en,COL.ptarget,0.3,.95);
+ const xt=sc.x(ca.rtTolerance);
+ E(sv,"line",{x1:xt,y1:box.t,x2:xt,y2:box.t+box.h,stroke:COL.warn,"stroke-width":1.5,"stroke-dasharray":"4 3"});
+ E(sv,"text",{class:"axtitle",x:xt+5,y:box.t+11,fill:COL.warn}).textContent="±"+trim(ca.rtTolerance)+" min";
+ const items=[{name:"target",color:COL.target}];
+ if(en)items.push({name:"entrapment",color:COL.ptarget});
+ items.push({name:"tolerance",color:COL.warn,line:true});
+ legend(document.getElementById("caHistLegend"),items);
+ document.getElementById("caHistNote").innerHTML=
+ "Each class is normalized to its own total. Apexes land on the acquisition scan grid, so two "+
+ "IDs on ONE feature differ by a whole number of cycles — the histogram combs rather than "+
+ "curving, and the excess decays into the flat chance-co-elution floor. Where it flattens is "+
+ "the tolerance, read off the data rather than asserted.";
+}
+
+// The rate at every tolerance on the ladder, from a single scan (each precursor's minimum |dRT|
+// to a better-scoring partner is retained, so a rate is a threshold count over those minima).
+function drawCaLadder(ca,s){
+ const host=document.getElementById("caLadder");host.innerHTML="";
+ const tol=ca.toleranceLadder||[];
+ if(!tol.length){host.innerHTML="No tolerance ladder.
";return;}
+ const series=[["target",s.target,COL.target],["entrapment",s.entrapment,COL.ptarget],
+ ["decoy",s.decoy,COL.decoy]].filter(x=>x[1]&&x[1].betterByTolerance);
+ const W=520,Hh=250,box={l:52,t:14,w:W-70,h:Hh-58};
+ let mx=0;series.forEach(x=>x[1].betterByTolerance.forEach(v=>{if(isFinite(v)&&v>mx)mx=v;}));
+ const sv=svg(W,Hh);host.appendChild(sv);
+ const sc=axes(sv,box,[tol[0],tol[tol.length-1]],[0,Math.max(mx*1.1,0.01)],
+ {xlab:"apex RT tolerance (min)",ylab:"outscored by a target",yfmt:v=>pct(v,0)});
+ series.forEach(x=>{let d="";const a=x[1].betterByTolerance;
+ tol.forEach((t,i)=>{if(!isFinite(a[i]))return;d+=(d?"L":"M")+sc.x(t).toFixed(1)+" "+sc.y(a[i]).toFixed(1)+" ";});
+ if(d)E(sv,"path",{d:d,fill:"none",stroke:x[2],"stroke-width":2});
+ tol.forEach((t,i)=>{if(isFinite(a[i]))E(sv,"circle",{cx:sc.x(t),cy:sc.y(a[i]),r:2.8,fill:x[2]});});});
+ const xt=sc.x(ca.rtTolerance);
+ E(sv,"line",{x1:xt,y1:box.t,x2:xt,y2:box.t+box.h,stroke:COL.warn,"stroke-width":1.5,"stroke-dasharray":"4 3"});
+ legend(document.getElementById("caLadderLegend"),
+ series.map(x=>({name:x[0],color:x[2]})).concat([{name:"headline tolerance",color:COL.warn,line:true}]));
+ // Table beside the chart: the ratio is the interpretable quantity, so give it a column.
+ const host2=document.getElementById("caLadderTable");host2.innerHTML="";
+ const tb=H(host2,"table"),th=H(H(tb,"thead"),"tr");
+ ["±RT (min)","Target","Entrapment","Enrichment"].forEach((h,i)=>H(th,"th",{class:i===0?"l":""},h));
+ const body=H(tb,"tbody");
+ tol.forEach((t,i)=>{const tr=H(body,"tr",{style:t===ca.rtTolerance?"font-weight:700":""});
+ H(tr,"td",{class:"l num"},trim(t));
+ const tv=s.target.betterByTolerance[i];
+ H(tr,"td",{class:"num"},pct(tv,2));
+ const ev=s.entrapment?s.entrapment.betterByTolerance[i]:NaN;
+ H(tr,"td",{class:"num"},s.entrapment?pct(ev,2):"–");
+ // Same MIN_N_FOR_ENRICHMENT gate the KPI and the class table apply. Without it this table
+ // printed a ratio for a population the two renderers above withhold as "n<30" - one
+ // suppressed headline beside five unsuppressed ratios for the same data. minNForEnrichment
+ // is serialized into the payload precisely so there is one definition, not one per renderer.
+ const caSuppressed=!s.entrapment||s.entrapment.n0)?ev/tv:NaN;
+ H(tr,"td",{class:"num"},caSuppressed?"n<"+ca.minNForEnrichment:(isFinite(r)?fmt(r,1)+"×":"–"));});
+}
+
+// One row per precursor PAIR (not per observation), ranked by score gap, with the run count.
+function drawCaOffenders(s){
+ const host=document.getElementById("caOffenders");host.innerHTML="";
+ const off=s.worstOffenders||[];
+ if(!off.length){host.innerHTML="No co-assigned pairs at the headline tolerance.
";return;}
+ const tb=H(host,"table"),th=H(H(tb,"thead"),"tr");
+ ["Precursor","Class","Better-scoring target on the same peak","z","m/z","Apex","ΔRT","Score gap","Runs"]
+ .forEach((h,i)=>H(th,"th",{class:(i===0||i===1||i===2)?"l":""},h));
+ const body=H(tb,"tbody");
+ off.forEach(o=>{const tr=H(body,"tr");
+ H(tr,"td",{class:"l num"},o.modifiedSequence);
+ const c=H(tr,"td",{class:"l"});
+ c.appendChild(document.createTextNode(o.class==="PTarget"?"entrapment":o.class.toLowerCase()));
+ if(o.sameBaseSequence)H(c,"span",{class:"pill",title:"same sequence once modifications are stripped - both IDs may be real"},"PTM isomer");
+ H(tr,"td",{class:"l num"},o.partnerModifiedSequence);
+ [String(o.charge),fmt(o.precursorMz,4),fmt(o.apexRt,2),fmt(o.deltaRt,3),fmt(o.scoreGap,2),fmt(o.runs,0)]
+ .forEach(v=>H(tr,"td",{class:"num"},v));});
+}
+
// ---------- summary: population KPIs (pass-independent, not q-gated) ----------
(function(){const k=document.getElementById("sumKpis");
kpi(k,"targets (best/prec)",fmt(D.nTarget,0),"");
diff --git a/pwiz_tools/Osprey/Osprey.Tasks/Pass2FdrSidecar.cs b/pwiz_tools/Osprey/Osprey.Tasks/Pass2FdrSidecar.cs
index 1edfd7fb50..b4cdb08cd8 100644
--- a/pwiz_tools/Osprey/Osprey.Tasks/Pass2FdrSidecar.cs
+++ b/pwiz_tools/Osprey/Osprey.Tasks/Pass2FdrSidecar.cs
@@ -185,7 +185,8 @@ internal static FeatureContributions ComputeAndPersist(
totalFiles++;
if (!inputByFileName.TryGetValue(kvp.Key, out string probeInput))
continue;
- if (!File.Exists(FdrScoresSidecar.Pass2Path(probeInput)))
+ if (!FdrScoresSidecar.IsCurrentFormat(FdrScoresSidecar.Pass2Path(probeInput),
+ FdrScoresSidecar.Pass.SecondPass))
missingPass2++;
}
if (missingPass2 > 0)
@@ -289,7 +290,7 @@ void FlushPass2File(string fileName, IReadOnlyList records)
if (!inputByFileName.TryGetValue(fileName, out string inputFileFlush))
return;
string pass2PathFlush = FdrScoresSidecar.Pass2Path(inputFileFlush);
- if (File.Exists(pass2PathFlush))
+ if (FdrScoresSidecar.IsCurrentFormat(pass2PathFlush, FdrScoresSidecar.Pass.SecondPass))
{
pass2Tally.AlreadyOnDisk++;
return;
@@ -419,7 +420,7 @@ void FlushPass2File(string fileName, IReadOnlyList records)
if (!inputByFileName.TryGetValue(fileName, out string inputFile3))
continue;
string pass2Path = FdrScoresSidecar.Pass2Path(inputFile3);
- if (File.Exists(pass2Path))
+ if (FdrScoresSidecar.IsCurrentFormat(pass2Path, FdrScoresSidecar.Pass.SecondPass))
{
pass2Tally.AlreadyOnDisk++;
continue;
@@ -504,7 +505,7 @@ void FlushPass2File(string fileName, IReadOnlyList records)
if (!inputByName.TryGetValue(kvp.Key, out string inputFile4))
continue;
string pass2Path = FdrScoresSidecar.Pass2Path(inputFile4);
- if (!File.Exists(pass2Path))
+ if (!FdrScoresSidecar.IsCurrentFormat(pass2Path, FdrScoresSidecar.Pass.SecondPass))
{
filesMissing++;
continue;
@@ -605,6 +606,13 @@ private static void RestorePass1Scalars(
pair.Key.Score = pair.Value.Score;
pair.Key.Pep = pair.Value.Pep;
pair.Key.RunProteinQvalue = pair.Value.RunProteinQvalue;
+ // The FOURTH field of the same five-of-eight gap (sidecar v4, issue
+ // #4522). ResetScores clears it with Score, and no frozen 2nd-pass mode
+ // writes it back, so it lands in the 2nd-pass sidecar at 0.0 for every
+ // peak Stage 6 touched. That is the whole population this method exists
+ // to repair, and it is why the seed should follow the record rather than
+ // an enumerated list: the list has now grown twice.
+ pair.Key.ExperimentAggregateScore = pair.Value.ExperimentAggregateScore;
}
filesRead++;
nRestored += staged.Count;
@@ -631,15 +639,15 @@ private static void RestorePass1Scalars(
if (unreadable.Count > 0)
{
ctx.LogWarning(string.Format(
- "1st-pass Score/Pep/RunProteinQvalue could not be restored for {0} file(s) " +
- "(no readable 1st-pass sidecar): [{1}]. Peaks Stage 6 changed in those files " +
- "keep reset defaults, so their 2nd-pass sidecars are wrong AND a Score of 0 " +
- "enters the second-pass protein FDR null unfiltered. Treat this run's " +
- "protein-level numbers as unreliable.",
+ "1st-pass Score/Pep/RunProteinQvalue/ExperimentAggregateScore could not be " +
+ "restored for {0} file(s) (no readable 1st-pass sidecar): [{1}]. Peaks Stage 6 " +
+ "changed in those files keep reset defaults, so their 2nd-pass sidecars are " +
+ "wrong AND a Score of 0 enters the second-pass protein FDR null unfiltered. " +
+ "Treat this run's protein-level numbers as unreliable.",
unreadable.Count, string.Join(", ", unreadable)));
}
ctx.LogVerbose(string.Format(
- "Restored 1st-pass Score/Pep/RunProteinQvalue onto {0} survivor(s) across {1} file(s).",
+ "Restored 1st-pass Score/Pep/RunProteinQvalue/ExperimentAggregateScore onto {0} survivor(s) across {1} file(s).",
nRestored, filesRead));
}
@@ -728,24 +736,37 @@ private static bool ComputePass2TransferCompeteFull(
perFileEntries.Count, StringComparer.Ordinal);
var survivorEntryIds = new HashSet();
long survivorObservations = 0;
- foreach (var kvp in perFileEntries)
+ // Reported because this walks EVERY survivor observation - 89,068,375 of them on the
+ // 82-file SEA-AD run - into a HashSet before anything downstream logs a word. It sat
+ // inside a 195 s silence between "Released library fragments" and the
+ // OSPREY_PASS2_QVALUE banner, which reads as a hung run at the very end of a
+ // multi-hour search. The two steps after it (sidecar path validation and the protein
+ // stratum build) are in the same silence and are NOT yet reported - see the TODO.
+ using (var mergeProgress = new ProgressReporter(
+ string.Format(@"Collecting pass-2 survivors from {0} file(s)", perFileEntries.Count),
+ perFileEntries.Count, string.Empty, ProgressReporter.IO_INTERVAL_SECONDS))
{
- if (entriesByFile.TryGetValue(kvp.Key, out var merged))
- {
- // New list, never AddRange onto the caller's: perFileEntries is the live
- // Stage 7 survivor buffer and must not gain entries as a side effect.
- var combined = new List(merged.Count + kvp.Value.Count);
- combined.AddRange(merged);
- combined.AddRange(kvp.Value);
- entriesByFile[kvp.Key] = combined;
- }
- else
+ int mergeIdx = 0;
+ foreach (var kvp in perFileEntries)
{
- entriesByFile[kvp.Key] = kvp.Value;
+ mergeProgress.Report(++mergeIdx);
+ if (entriesByFile.TryGetValue(kvp.Key, out var merged))
+ {
+ // New list, never AddRange onto the caller's: perFileEntries is the live
+ // Stage 7 survivor buffer and must not gain entries as a side effect.
+ var combined = new List(merged.Count + kvp.Value.Count);
+ combined.AddRange(merged);
+ combined.AddRange(kvp.Value);
+ entriesByFile[kvp.Key] = combined;
+ }
+ else
+ {
+ entriesByFile[kvp.Key] = kvp.Value;
+ }
+ survivorObservations += kvp.Value.Count;
+ foreach (var e in kvp.Value)
+ survivorEntryIds.Add(e.EntryId);
}
- survivorObservations += kvp.Value.Count;
- foreach (var e in kvp.Value)
- survivorEntryIds.Add(e.EntryId);
}
// 2. Per-file scalar sidecar paths. Validate every sidecar up front so we fail fast
@@ -772,6 +793,20 @@ private static bool ComputePass2TransferCompeteFull(
ctx.LogWarning("transfer-compete: 1st-pass scalar sidecar not found: " + sidecarPath);
return false;
}
+ // Existence was never enough. ReadScalars THROWS on bad magic, a stale version, a
+ // wrong pass byte or a partial record, and its only call site is inside the
+ // streaming closure below - which has already written e.Score = frozenScore for
+ // files 1..N by the time file N+1 is rejected. That aborts a multi-hour run on a
+ // raw IOException with the survivor pool half-mutated, contradicting this method's
+ // own contract that "every return false is placed BEFORE any survivor is mutated".
+ // Checking the header here keeps the refusal where the contract says it is.
+ if (!FdrScoresSidecar.IsCurrentFormat(sidecarPath, FdrScoresSidecar.Pass.FirstPass))
+ {
+ ctx.LogWarning(
+ "transfer-compete: 1st-pass scalar sidecar is not a readable v" +
+ FdrScoresSidecar.FormatVersion + " first-pass file: " + sidecarPath);
+ return false;
+ }
fileKeys.Add(kvp.Key);
sidecarByKey[kvp.Key] = sidecarPath;
}
@@ -919,7 +954,8 @@ private static bool ComputePass2TransferCompeteFull(
}
nScored += fileScores.Count;
- FdrScoresSidecar.ReadScalars(sidecarByKey[fileKey], out uint[] eids, out double[] scs);
+ FdrScoresSidecar.ReadScalars(sidecarByKey[fileKey], FdrScoresSidecar.Pass.FirstPass,
+ out uint[] eids, out double[] scs);
if (stratumBaseIds != null)
StashOffStratumPass1ExperimentQ(fileKey, sidecarByKey[fileKey], eids, scs, fileScores);
progress.Report(++nRead);
@@ -960,14 +996,25 @@ void StashOffStratumPass1ExperimentQ(string fileKey, string sidecarPath,
}
if (wanted.Count == 0)
return;
- FdrScoresSidecar.ReadRecords(sidecarPath, FdrScoresSidecar.Pass.FirstPass, rec =>
+ // The result matters here as much as at the other read sites: ReadRecords
+ // returns false AFTER invoking the callback, so a partial read leaves
+ // pass1ExpQByKey holding SOME of this file's off-stratum q-values. Those are
+ // carried forward verbatim by the off-stratum branch, so a silent partial
+ // fill gives a subset of survivors their pass-1 q and the rest a default -
+ // a per-entry mix no downstream check can see.
+ if (!FdrScoresSidecar.ReadRecords(sidecarPath, FdrScoresSidecar.Pass.FirstPass, rec =>
{
if (wanted.Contains(rec.EntryId))
{
pass1ExpQByKey[(fileKey, rec.EntryId)] =
(rec.ExperimentPrecursorQvalue, rec.ExperimentPeptideQvalue);
}
- });
+ }))
+ {
+ throw new IOException(
+ @"1st-pass sidecar could not be read in full while stashing off-stratum experiment q-values: " +
+ sidecarPath);
+ }
}
competition = StreamingFdr.ComputeFullPopulationPrecursorFdrStreaming(
@@ -1009,6 +1056,21 @@ void StashOffStratumPass1ExperimentQ(string fileKey, string sidecarPath,
// reported set (peptide-level FDR is not the target here).
e.ExperimentPeptideQvalue = eq;
e.Pep = competition.Pep(kvp.Key, e.EntryId);
+ // The aggregate MUST move with the q above. This mode recomputes experiment q
+ // from a fresh full-population competition, so the pass-1 aggregate
+ // RestorePass1Scalars seeded is no longer the score that q was ranked on -
+ // and this is the DEFAULT mode, so leaving it stale is not an edge case.
+ // Measured cost of the omission: the co-assignment panel's experiment
+ // boundary is a minimum over accepted precursors' aggregates, so entries
+ // still holding the ResetScores 0.0 default dragged it to 0.0 and admitted
+ // the entire decoy pool - 542,368 decoys against 117,783 targets on astral,
+ // 183x the pass-1 count, from a rule meant to admit about 1%.
+ // null means the entry never entered the experiment fold (off-stratum under
+ // protein-compact); those keep the pass-1 value, which is correct because
+ // they keep the pass-1 experiment q too - the branch above.
+ double? agg = competition.ExperimentAggregateScore(e.EntryId);
+ if (agg.HasValue)
+ e.ExperimentAggregateScore = agg.Value;
nMapped++;
}
ctx.LogInfo(string.Format(
@@ -1542,6 +1604,14 @@ internal static bool TransferPerRunQ(
// retrain -- hard-fail over warn-and-proceed on silently-invalid output.
var globalExpPrecQ = new Dictionary();
var globalExpPepQ = new Dictionary();
+ // The experiment aggregate score belongs with the experiment q above: it is the score
+ // that q's competition ranked on, and a gap-fill that takes one without the other
+ // persists a q paired with a score it was never computed from - the exact pairing this
+ // field exists to guarantee. Reduced by MAX rather than the q-values' MIN because
+ // higher is better here, and because 0.0 is FdrEntry.ResetScores' default and sits mid
+ // distribution for a signed discriminant, so max also keeps a reset stub from
+ // displacing a real negative score.
+ var globalExpAgg = new Dictionary();
// Per-file progress: reading every file's 1st-pass sidecar ran silently for minutes on
// an 82-file join. Console-only; disposed on every exit (including the fallback return).
using (var scanProgress = new ProgressReporter(
@@ -1564,6 +1634,17 @@ internal static bool TransferPerRunQ(
if (!globalExpPepQ.TryGetValue(rec.EntryId, out double curPep) ||
rec.ExperimentPeptideQvalue < curPep)
globalExpPepQ[rec.EntryId] = rec.ExperimentPeptideQvalue;
+ // Prefer a REAL aggregate over the 0.0 ResetScores default, rather than
+ // taking the max - 0.0 sits above 93-99% of measured aggregates, so a max
+ // would let a single default row outrank every real (negative) one for
+ // this entry. Same rule, and the same reasoning, as
+ // CoAssignmentAccumulator.ObserveCutoff; see the comment there for the
+ // measurement. No 1st-pass sidecar record carries a 0.0 today (0 of 24.7M
+ // over six SEA-AD files), so this is prophylactic here and essential
+ // there, where the pass-2 pool does carry stubs.
+ if (!globalExpAgg.TryGetValue(rec.EntryId, out double curAgg) || curAgg == 0.0 ||
+ (rec.ExperimentAggregateScore != 0.0 && rec.ExperimentAggregateScore > curAgg))
+ globalExpAgg[rec.EntryId] = rec.ExperimentAggregateScore;
});
if (!readOk)
{
@@ -1641,8 +1722,12 @@ internal static bool TransferPerRunQ(
// them at the precursor's best-run q; a precursor with no record anywhere -> 1.
double gapExpPrecQ = globalExpPrecQ.TryGetValue(entry.EntryId, out double gPrec) ? gPrec : 1.0;
double gapExpPepQ = globalExpPepQ.TryGetValue(entry.EntryId, out double gPep) ? gPep : 1.0;
+ // 0.0 when the precursor has no record anywhere, which pairs with the q = 1.0
+ // above: never competed, never accepted, so nothing reads it.
+ double gapExpAgg = globalExpAgg.TryGetValue(entry.EntryId, out double gAgg) ? gAgg : 0.0;
switch (AssignPerRunQ(entry, newScore, rec1,
- precScoresDesc, precQDesc, pepScoresDesc, pepQDesc, gapExpPrecQ, gapExpPepQ))
+ precScoresDesc, precQDesc, pepScoresDesc, pepQDesc,
+ gapExpPrecQ, gapExpPepQ, gapExpAgg))
{
case PerRunClass.Unchanged: nUnchanged++; break;
case PerRunClass.Moved: nMoved++; break;
@@ -1691,7 +1776,9 @@ internal enum PerRunClass
/// full 1st-pass record verbatim.
/// - MOVED: run q re-mapped from the tables; experiment q + PEP carried from the record.
/// - GAP-FILL (no record): run q from the tables; experiment q =
- /// / .
+ /// / , and the
+ /// experiment aggregate score = from the same cross-file
+ /// source, so the persisted score and the q it ranked for stay paired.
///
///
internal static PerRunClass AssignPerRunQ(
@@ -1703,7 +1790,8 @@ internal static PerRunClass AssignPerRunQ(
double[] pepScoresDesc,
double[] pepQDesc,
double gapFillExpPrecQ,
- double gapFillExpPepQ)
+ double gapFillExpPepQ,
+ double gapFillExpAgg)
{
if (firstPass.HasValue)
{
@@ -1721,6 +1809,7 @@ internal static PerRunClass AssignPerRunQ(
entry.ExperimentPrecursorQvalue = rec1.ExperimentPrecursorQvalue;
entry.ExperimentPeptideQvalue = rec1.ExperimentPeptideQvalue;
entry.Pep = rec1.Pep;
+ entry.ExperimentAggregateScore = rec1.ExperimentAggregateScore;
return PerRunClass.Unchanged;
}
entry.Score = newScore;
@@ -1730,6 +1819,11 @@ internal static PerRunClass AssignPerRunQ(
entry.ExperimentPrecursorQvalue = rec1.ExperimentPrecursorQvalue;
entry.ExperimentPeptideQvalue = rec1.ExperimentPeptideQvalue;
entry.Pep = rec1.Pep;
+ // Carried with the experiment q for the same reason, and NOT re-derived from
+ // newScore: it is the score that pass-1 experiment q was computed from, so
+ // re-mapping it to the rescored value would break the pairing that is the
+ // whole point of persisting it.
+ entry.ExperimentAggregateScore = rec1.ExperimentAggregateScore;
return PerRunClass.Moved;
}
entry.Score = newScore;
@@ -1737,6 +1831,13 @@ internal static PerRunClass AssignPerRunQ(
entry.RunPeptideQvalue = LookupQForScore(newScore, pepScoresDesc, pepQDesc);
entry.ExperimentPrecursorQvalue = gapFillExpPrecQ;
entry.ExperimentPeptideQvalue = gapFillExpPepQ;
+ // Carried for the same reason as the experiment q beside it, and from the same
+ // cross-file source: the aggregate is a per-entry roll-up, identical in every file's
+ // record for that entry, so a gap-fill is entitled to it even with no record of its
+ // own. Leaving it at ResetScores' 0.0 would persist a real experiment q next to a
+ // score that q was not computed from, and a score-space acceptance boundary built
+ // from the 2nd-pass sidecar would then be drawn from the wrong ranking.
+ entry.ExperimentAggregateScore = gapFillExpAgg;
return PerRunClass.GapFill;
}
diff --git a/pwiz_tools/Osprey/Osprey.Tasks/PerFileRescoreTask.cs b/pwiz_tools/Osprey/Osprey.Tasks/PerFileRescoreTask.cs
index ece0da43c0..e817af5a1d 100644
--- a/pwiz_tools/Osprey/Osprey.Tasks/PerFileRescoreTask.cs
+++ b/pwiz_tools/Osprey/Osprey.Tasks/PerFileRescoreTask.cs
@@ -193,7 +193,13 @@ public override string ValidityKey(PipelineContext ctx)
// The Stage 6 handoff arm joins them: the streamed and resident arms are supposed
// to write byte-identical reconciled parquets, and an in-place A/B that silently
// adopted the other arm's outputs would report that identity without testing it.
+ // The sidecar format version belongs here too, not only in FirstPassFdrTask: this
+ // task WRITES the 2nd-pass sidecar, so a record-layout change (v3 -> v4) invalidates
+ // its output exactly as it invalidates the 1st-pass one. Without it, FirstPassFDR
+ // re-ran and rewrote v4 while this task and SecondPassFDR considered themselves
+ // valid against v3 files.
return base.ValidityKey(ctx)
+ + @";fdrsidecar=" + FdrScoresSidecar.FormatVersion
+ @";reconciliation=" + ctx.Config.Identity.ReconciliationParameterHash()
+ OspreyEnvironment.ExperimentAggValidityKeySuffix()
+ OspreyEnvironment.Pass2QValueValidityKeySuffix()
@@ -255,7 +261,12 @@ public override bool Run(PipelineContext ctx)
{
foreach (var inputFile in ctx.Config.InputFiles)
{
- if (File.Exists(FdrScoresSidecar.Pass2Path(inputFile)))
+ // Presence is not readability. A bare File.Exists cannot see a version, so a
+ // sidecar left by a build before the v3 -> v4 record change satisfied this
+ // gate and made the WHOLE Stage 6 rescore a no-op - the run then finished
+ // green carrying 1st-pass q-values into the picked-protein FDR and the .blib.
+ if (FdrScoresSidecar.IsCurrentFormat(FdrScoresSidecar.Pass2Path(inputFile),
+ FdrScoresSidecar.Pass.SecondPass))
{
anyPass2Present = true;
break;
diff --git a/pwiz_tools/Osprey/Osprey.Tasks/PerFileScoringTask.cs b/pwiz_tools/Osprey/Osprey.Tasks/PerFileScoringTask.cs
index c2da54a37e..c9740f4dd1 100644
--- a/pwiz_tools/Osprey/Osprey.Tasks/PerFileScoringTask.cs
+++ b/pwiz_tools/Osprey/Osprey.Tasks/PerFileScoringTask.cs
@@ -381,7 +381,21 @@ public override bool Run(PipelineContext ctx)
// resident-pool consumer FDRBench pass 1, which walks the full pre-compaction
// FdrEntry pool -- still needs the fat stubs here. --model-diagnostics is NOT
// one of them any more (#4505): it streams its report on every path.
- bool needsResidentPool = NeedsResidentPool(ctx.Config);
+ // The fat/lean decision, and the guard that checks it, key off CanUseLeanProjection -
+ // the same predicate the two sibling sites use. Bare NeedsResidentPool no longer
+ // excludes ExpectReconciledInput (#4486), so a config with it set reached here with
+ // needsResidentPool == false: GuardResidentPool was handed false and refused nothing,
+ // and the lean branch streamed projection rows with no FdrEntry allocated, handing
+ // Stage 7 empty per-file lists. That is precisely what ResidentPoolGuardTest asserts
+ // must never happen.
+ //
+ // NOT reachable today, and the claim that it was is wrong: Program.cs rejects
+ // --task SecondPassFDR combined with --input and requires --input-scores, so
+ // ExpectReconciledInput implies InputScores.Count > 0 and IsIncluded returns false -
+ // Run is never entered on that config. This is aligned with its two siblings so the
+ // one decision has one predicate, not so that a live defect is closed.
+ bool needsResidentPool = !CanUseLeanProjection(ctx.Config, hasReconSidecars: false,
+ OspreyEnvironment.UseFdrProjection);
GuardResidentPool(ctx.Config, needsResidentPool);
FdrProjectionSet projections = null;
@@ -1751,8 +1765,10 @@ private bool HydrateRescoreBundleIfPresent(
if (!streamed)
{
foreach (var kvp in perFileEntries)
+ {
foreach (var entry in kvp.Value)
entry.Features = null;
+ }
}
ctx.LogInfo(string.Format(
@"Hydrated rescore bundle for {0} file(s) ({1} reconciliation actions, " +
@@ -1808,7 +1824,11 @@ private static bool AllHaveReconSidecars(OspreyConfig config)
foreach (var parquetPath in config.InputScores)
{
string syntheticInput = RescoreHydration.SyntheticInputFromParquet(parquetPath);
- if (!File.Exists(FdrScoresSidecar.Pass1Path(syntheticInput))
+ // Version-fenced like every other sidecar gate: a v3 file left by an older build
+ // is present but unreadable, and answering "yes, all sidecars are here" off
+ // File.Exists keeps the fat pool on a path whose overlay then cannot load it.
+ if (!FdrScoresSidecar.IsCurrentFormat(FdrScoresSidecar.Pass1Path(syntheticInput),
+ FdrScoresSidecar.Pass.FirstPass)
|| !File.Exists(ReconciliationFile.PathForInput(syntheticInput)))
return false;
}
diff --git a/pwiz_tools/Osprey/Osprey.Tasks/ScoringTaskShared.cs b/pwiz_tools/Osprey/Osprey.Tasks/ScoringTaskShared.cs
index aba9482318..9e400135fd 100644
--- a/pwiz_tools/Osprey/Osprey.Tasks/ScoringTaskShared.cs
+++ b/pwiz_tools/Osprey/Osprey.Tasks/ScoringTaskShared.cs
@@ -189,6 +189,24 @@ internal static SpectraWindowIndex EnsureSpectraCache(string inputFile, bool ser
try
{
SpectraCache.SaveSpectraCache(cachePath, mzmlResult.Ms2Spectra, mzmlResult.Ms1Spectra, inputFile);
+ // Name the file that was just written, the way the library cache does. Without
+ // this the only evidence a multi-GB cache was produced is a silent gap in the
+ // log, and nothing says WHERE it landed - which matters because --work-dir
+ // redirects this path away from the data directory (ArtifactPaths.ResolveCacheDir),
+ // so a run can rebuild caches that already exist beside the mzML.
+ long cacheBytes = 0;
+ try
+ {
+ if (File.Exists(cachePath))
+ cacheBytes = new FileInfo(cachePath).Length;
+ }
+ catch
+ {
+ cacheBytes = 0;
+ }
+ ctx.LogInfo(string.Format("Saved spectra cache ({0} MS2 + {1} MS1, {2:F2} GB) to '{3}'",
+ mzmlResult.Ms2Spectra.Count, mzmlResult.Ms1Spectra.Count,
+ cacheBytes / 1024.0 / 1024.0 / 1024.0, cachePath));
}
catch (Exception ex)
{
diff --git a/pwiz_tools/Osprey/Osprey.Tasks/SecondPassFdrTask.cs b/pwiz_tools/Osprey/Osprey.Tasks/SecondPassFdrTask.cs
index bf0450f130..0ac0b17118 100644
--- a/pwiz_tools/Osprey/Osprey.Tasks/SecondPassFdrTask.cs
+++ b/pwiz_tools/Osprey/Osprey.Tasks/SecondPassFdrTask.cs
@@ -84,6 +84,30 @@ public override IEnumerable Outputs(PipelineContext ctx)
{
if (!string.IsNullOrEmpty(ctx.Config.OutputBlib))
yield return ctx.Config.OutputBlib;
+ // The --model-diagnostics report is an output of THIS task (it is finalized in
+ // WritePass2AndFinalize), so declare it and let ordinary task validity regenerate
+ // it. Task validity requires every declared output to exist, so a deleted or
+ // renamed report invalidates this task alone - Stages 1-5 stay cached and the
+ // pass-1 panel is rebuilt by rehydrating the 1st-pass sidecars, the same path
+ // regression mode 5 already covers.
+ //
+ // CONDITIONAL ON THE FLAG, deliberately. Declaring it unconditionally would make
+ // every run that never asked for diagnostics permanently invalid, re-running
+ // SecondPassFDR forever.
+ //
+ // Without this the flag was inert on a completed directory: --model-diagnostics is
+ // in no validity key and the HTML was in no Outputs list, so adding it to a re-run
+ // changed nothing, every task reported "outputs valid", and no report was produced.
+ //
+ // AND conditional on FirstPassFDR being in this graph. WritePass2AndFinalize needs
+ // the pass-1 .data.json hand-off sidecar, which only FirstPassFdrTask writes - so
+ // under `--task SecondPassFDR --model-diagnostics` no report can ever be produced.
+ // Declaring it there recreated the very loop the paragraph above set out to avoid,
+ // one step later: CanRehydrate requires every declared output to exist, so the task
+ // was never skippable and every invocation re-ran pass-2 Percolator, protein FDR and
+ // the whole .blib write, still producing no report.
+ if (ctx.Config.ModelDiagnostics && FirstPassFdrTask.IsIncludedFor(ctx.Config))
+ yield return ModelDiagnosticsReport.ReportPath(ctx.Config);
// 2nd-pass FDR sidecars are written whenever Stage 6 rescored entries
// (independent of protein FDR -- the second Percolator pass runs on the
// reconciled features), so declare them on that same condition.
@@ -104,7 +128,10 @@ public override string ValidityKey(PipelineContext ctx)
// The 2nd-pass mode decides the q this task writes into the .blib and the 2nd-pass
// sidecars, so it invalidates them by exactly the argument the aggregation suffix
// makes above - one arm's .blib must never be reused as another's.
+ // And the sidecar format version, for the same reason FirstPassFdrTask carries it:
+ // this task writes the 2nd-pass sidecars, so a record-layout change invalidates them.
return base.ValidityKey(ctx)
+ + @";fdrsidecar=" + FdrScoresSidecar.FormatVersion
+ @";reconciliation=" + ctx.Config.Identity.ReconciliationParameterHash()
+ OspreyEnvironment.ExperimentAggValidityKeySuffix()
+ OspreyEnvironment.Pass2QValueValidityKeySuffix()
diff --git a/pwiz_tools/Osprey/Osprey.Test/FdrTest.cs b/pwiz_tools/Osprey/Osprey.Test/FdrTest.cs
index 8b8bff63b4..3b4841e78e 100644
--- a/pwiz_tools/Osprey/Osprey.Test/FdrTest.cs
+++ b/pwiz_tools/Osprey/Osprey.Test/FdrTest.cs
@@ -955,21 +955,25 @@ public void TestFdrProjectionRoundTripsFields()
///
/// Minimal for the projection parity tests: records
- /// each row's Score + by (fileIdx, rowIdx) so the test
- /// can compare the streamed outputs against the FdrEntry oracle now that the lean
- /// struct no longer stores them (issue #4355 struct-shrink S0).
+ /// each row's Score + experiment aggregate score + by
+ /// (fileIdx, rowIdx) so the test can compare the streamed outputs against the
+ /// FdrEntry oracle now that the lean struct no longer stores them (issue #4355
+ /// struct-shrink S0).
///
private sealed class CapturingSink : IFdrOutputSink
{
private readonly Dictionary<(int, int), double> _scores = new Dictionary<(int, int), double>();
+ private readonly Dictionary<(int, int), double> _expAgg = new Dictionary<(int, int), double>();
private readonly Dictionary<(int, int), FdrQValues> _q = new Dictionary<(int, int), FdrQValues>();
private readonly Dictionary<(int, int), (uint EntryId, bool IsDecoy, byte Charge, string Peptide)> _ident =
new Dictionary<(int, int), (uint, bool, byte, string)>();
public void Accept(int fileIdx, int rowIdx, uint entryId, bool isDecoy,
- byte charge, string peptide, double score, in FdrQValues q)
+ byte charge, string peptide, double score, double experimentAggregateScore,
+ in FdrQValues q)
{
_scores[(fileIdx, rowIdx)] = score;
+ _expAgg[(fileIdx, rowIdx)] = experimentAggregateScore;
_q[(fileIdx, rowIdx)] = q;
_ident[(fileIdx, rowIdx)] = (entryId, isDecoy, charge, peptide);
}
@@ -979,6 +983,7 @@ public void Finish(Action logInfo)
}
public double ScoreAt(int fileIdx, int rowIdx) => _scores[(fileIdx, rowIdx)];
+ public double ExperimentAggregateScoreAt(int fileIdx, int rowIdx) => _expAgg[(fileIdx, rowIdx)];
public FdrQValues QAt(int fileIdx, int rowIdx) => _q[(fileIdx, rowIdx)];
public (uint EntryId, bool IsDecoy, byte Charge, string Peptide) IdentAt(int fileIdx, int rowIdx)
=> _ident[(fileIdx, rowIdx)];
@@ -2203,6 +2208,25 @@ public void TestStreamingFirstPassQMatchesFlat()
AssertMapsEqual(
PercolatorQValues.ComputePepWinnerMap(scoreArr, labelArr, entryIdArr),
streaming.BuildPepWinnerMap(), "pep-winner");
+
+ // The score persisted beside those q-values (sidecar v4, issue #4522). The two
+ // paths derive it independently - the flat one by reducing the score array, the
+ // streaming one off the per-(base_id, side) bests it already keeps - so this is
+ // the check that they cannot disagree about what the experiment scope competed on.
+ var flatAgg = PercolatorQValues.ComputeExperimentAggregateScoreMap(
+ scoreArr, labelArr, entryIdArr, applyExperimentAgg: false);
+ AssertMapsEqual(flatAgg, streaming.BuildExperimentAggregateScoreMap(), "exp-aggregate");
+
+ // Under the default aggregation the aggregate IS the max over the entry's rows.
+ // Computed here straight from the fixture rather than from either implementation,
+ // so both are pinned to the definition instead of to each other.
+ var expectedMax = new Dictionary();
+ for (int i = 0; i < scoreArr.Length; i++)
+ {
+ if (!expectedMax.TryGetValue(entryIdArr[i], out double cur) || scoreArr[i] > cur)
+ expectedMax[entryIdArr[i]] = scoreArr[i];
+ }
+ AssertMapsEqual(expectedMax, flatAgg, "exp-aggregate-vs-definition");
}
///
@@ -2293,6 +2317,17 @@ private static void AssertStreamingMeanBestNMatchesResident(int n)
ResidentMeanBestNPeptideQMap(scoreArr, labelArr, entryIdArr, peptideArr, n),
streaming.BuildExperimentPeptideQMap(), "mbN exp-peptide");
+ // The persisted aggregate (sidecar v4, issue #4522) must follow the aggregation
+ // too - under mean(best-N) it is the group's mean-best-N score, NOT the raw max.
+ // This is the case the whole format change exists for: a consumer that rebuilt the
+ // roll-up with max() would be wrong here and right everywhere else.
+ var residentAgg = new Dictionary();
+ var aggPerRow = TargetDecoyCompetition.ComputeBaseIdMeanBestN(
+ scoreArr, labelArr, entryIdArr, n);
+ for (int i = 0; i < aggPerRow.Length; i++)
+ residentAgg[entryIdArr[i]] = aggPerRow[i];
+ AssertMapsEqual(residentAgg, streaming.BuildExperimentAggregateScoreMap(), "mbN exp-aggregate");
+
// PEP must still be the RAW-max map, untouched by the aggregation. That invariant
// rests entirely on the mean-best-N block in Add() sitting AFTER the _precTargets /
// _precDecoys update and ending in an unconditional return - hoisting it would leave
@@ -2713,7 +2748,12 @@ private static Dictionary ResidentMeanBestNPrecursorQMap(
PercolatorQValues.ComputeConservativeQvalues(ws, wd, q);
var map = new Dictionary(wi.Length);
for (int rank = 0; rank < wi.Length; rank++)
- map[entryIds[wi[rank]] & 0x7FFFFFFFu] = q[rank];
+ {
+ // Keyed by the WINNER's full entry_id, decoy bit intact - mirroring
+ // PercolatorQValues.ComputeExperimentPrecursorQMap. On base_id the losing side of
+ // each competition inherited the winner's q, which is the defect this pins shut.
+ map[entryIds[wi[rank]]] = q[rank];
+ }
return map;
}
diff --git a/pwiz_tools/Osprey/Osprey.Test/IOTest.cs b/pwiz_tools/Osprey/Osprey.Test/IOTest.cs
index e4f0c5f945..4a87878a3f 100644
--- a/pwiz_tools/Osprey/Osprey.Test/IOTest.cs
+++ b/pwiz_tools/Osprey/Osprey.Test/IOTest.cs
@@ -3162,23 +3162,29 @@ public void TestFdrScoresSidecarTwoPhasePatchMatchesSinglePhase()
try
{
// Finalized records with a distinct real run_protein_qvalue each (the
- // last arg). entry_ids are non-sequential so a positional patch would
- // land the wrong value.
+ // second-to-last arg). entry_ids are non-sequential so a positional patch
+ // would land the wrong value. Each record also carries a DISTINCT
+ // experiment_aggregate_score in the trailing [60..68] field, which the patch
+ // must leave untouched -- a patch that miscomputed the record stride would
+ // corrupt it and break the byte comparison below.
var real = new List
{
- new FdrScoreRecord(10, -3.5, 0.001, 0.0011, 0.0012, 0.0013, 0.02, 0.0042),
- new FdrScoreRecord(7, -3.4, 0.002, 0.0021, 0.0022, 0.0023, 0.05, 0.0123),
- new FdrScoreRecord(42, -3.3, 0.003, 0.0031, 0.0032, 0.0033, 0.08, 0.95),
- new FdrScoreRecord(3, -3.2, 0.004, 0.0041, 0.0042, 0.0043, 0.11, 1.0),
+ new FdrScoreRecord(10, -3.5, 0.001, 0.0011, 0.0012, 0.0013, 0.02, 0.0042, -1.25),
+ new FdrScoreRecord(7, -3.4, 0.002, 0.0021, 0.0022, 0.0023, 0.05, 0.0123, -0.75),
+ new FdrScoreRecord(42, -3.3, 0.003, 0.0031, 0.0032, 0.0033, 0.08, 0.95, 0.5),
+ new FdrScoreRecord(3, -3.2, 0.004, 0.0041, 0.0042, 0.0043, 0.11, 1.0, 2.125),
};
- // Phase-1 partial records: identical EXCEPT run_protein_qvalue = 1.0.
+ // Phase-1 partial records: identical EXCEPT run_protein_qvalue = 1.0. The
+ // aggregate score is already final at phase 1 (it comes from the score pass,
+ // not from protein FDR), so it is carried through unchanged.
var partial = new List(real.Count);
foreach (var r in real)
{
partial.Add(new FdrScoreRecord(
r.EntryId, r.Score, r.RunPrecursorQvalue, r.RunPeptideQvalue,
- r.ExperimentPrecursorQvalue, r.ExperimentPeptideQvalue, r.Pep, 1.0));
+ r.ExperimentPrecursorQvalue, r.ExperimentPeptideQvalue, r.Pep, 1.0,
+ r.ExperimentAggregateScore));
}
// Map entry_id -> finalized run_protein_qvalue, inserted out of record
diff --git a/pwiz_tools/Osprey/Osprey.Test/MeanBestNAggregationTest.cs b/pwiz_tools/Osprey/Osprey.Test/MeanBestNAggregationTest.cs
index 49a39b3a5b..84c2cccec6 100644
--- a/pwiz_tools/Osprey/Osprey.Test/MeanBestNAggregationTest.cs
+++ b/pwiz_tools/Osprey/Osprey.Test/MeanBestNAggregationTest.cs
@@ -456,7 +456,9 @@ private static void AssertWrappersMatchMaps(
scores, labels, entryIds, applyExperimentAgg);
for (int i = 0; i < scores.Length; i++)
{
- double expected = precMap.TryGetValue(entryIds[i] & BASE_ID_MASK, out double q) ? q : 1.0;
+ // Full entry_id, NOT base_id: the map is keyed by the WINNER of each target/decoy
+ // competition, so the losing side keeps 1.0 rather than inheriting the winner's q.
+ double expected = precMap.TryGetValue(entryIds[i], out double q) ? q : 1.0;
Assert.AreEqual(expected, precRows[i], 0.0,
string.Format(@"{0}: precursor wrapper row {1} must equal its map entry", which, i));
}
diff --git a/pwiz_tools/Osprey/Osprey.Test/ModelDiagnosticsDataTest.cs b/pwiz_tools/Osprey/Osprey.Test/ModelDiagnosticsDataTest.cs
index 6d6ee318ad..7d9b4d303f 100644
--- a/pwiz_tools/Osprey/Osprey.Test/ModelDiagnosticsDataTest.cs
+++ b/pwiz_tools/Osprey/Osprey.Test/ModelDiagnosticsDataTest.cs
@@ -61,6 +61,335 @@ public void TestModelDiagnosticsData()
TestPassingSetHonorsFdrLevel();
TestCalibrationBuildCalFile();
TestStreamingAccumulatorMatchesBatch();
+ TestPeakCoAssignment();
+ }
+
+ // Single-peak multiple-ID co-assignment (issue #4522) on a fixture where every reported
+ // number is derived by hand. Two runs; m/z and apex RT chosen so each pair lands
+ // unambiguously inside one |dRT| histogram bin (bin width 0.005 min), well clear of the
+ // edges, so the assertions do not depend on floating-point luck.
+ //
+ // file1 m/z apex score q
+ // A z2 target 500.000 10.000 9.0 ok the strong explanation
+ // A z2 target (dup) 500.000 10.150 2.0 ok pre-compaction second peak
+ // A z3 target 333.670 10.000 5.0 ok SAME sequence, other charge
+ // B z2 target 500.004 10.018 3.0 ok co-assigned, A outscores it
+ // E z2 entrapment 500.005 10.032 4.0 ok co-assigned, A outscores it
+ // X z2 decoy 500.006 10.008 6.0 ok above the acceptance score; A outscores it
+ // F z2 target 500.002 10.010 20.0 FAIL q-failing; must not partner
+ // file2
+ // A z2 target 500.000 30.000 6.0 ok same precursor, no partner
+ // C z2 target 700.000 20.000 8.0 ok partner 0.202 min away
+ // D z2 target 700.005 20.202 2.0 ok C outscores it, but far in RT
+ private static void TestPeakCoAssignment()
+ {
+ var mz = new Dictionary
+ {
+ { 1, 500.000 }, { 6, 333.670 }, { 2, 500.004 }, { 3, 700.000 },
+ { 4, 700.005 }, { 7, 500.002 }, { 201, 500.005 }, { 5 | DECOY_BIT, 500.006 },
+ };
+ var cls = new Dictionary
+ {
+ { 1, EntrapmentClass.Target }, { 2, EntrapmentClass.Target },
+ { 3, EntrapmentClass.Target }, { 4, EntrapmentClass.Target },
+ { 6, EntrapmentClass.Target }, { 7, EntrapmentClass.Target },
+ { 201, EntrapmentClass.PTarget },
+ };
+ var f1 = new List
+ {
+ // expAgg is a per-ENTRY cross-run roll-up, so every row of entry 1 carries 9.0
+ // (its best observation anywhere), not that row's own score.
+ CoEntry(1, false, 9.0, 0.001, "A", 2, 10.000, 9.0),
+ CoEntry(1, false, 2.0, 0.001, "A", 2, 10.150, 9.0),
+ CoEntry(6, false, 5.0, 0.001, "A", 3, 10.000, 5.0),
+ CoEntry(2, false, 3.0, 0.002, "B", 2, 10.018, 3.0),
+ CoEntry(201, false, 4.0, 0.003, "E", 2, 10.032, 4.0),
+ // The discriminating row. Its per-run SCORE (6.0) clears file1's run boundary
+ // (3.0, the worst accepted target there), so it is admitted at RUN scope. Its
+ // EXPERIMENT AGGREGATE (1.0) is below the experiment boundary (2.0, entry D's
+ // aggregate), so it must NOT be admitted at experiment scope. The two scopes
+ // therefore disagree about this one row, and they can only disagree if the
+ // experiment boundary reads ExperimentAggregateScore rather than Score.
+ CoEntry(5 | DECOY_BIT, true, 6.0, 0.004, "X", 2, 10.008, 1.0),
+ CoEntry(7, false, 20.0, 0.500, "F", 2, 10.010, 20.0),
+ };
+ var f2 = new List
+ {
+ CoEntry(1, false, 6.0, 0.001, "A", 2, 30.000, 9.0),
+ CoEntry(3, false, 8.0, 0.001, "C", 2, 20.000, 8.0),
+ CoEntry(4, false, 2.0, 0.002, "D", 2, 20.202, 2.0),
+ };
+
+ var data = ModelDiagnosticsData.BuildCoAssignment(
+ WrapFiles(f1, f2), cls, id => mz.TryGetValue(id, out double v) ? v : double.NaN,
+ 0.01, FdrLevel.Precursor, 1, false);
+ Assert.IsNotNull(data);
+ // "Detected" is reported at both q scopes; the fixture sets run and experiment q
+ // equal, so both scopes see the same rows and the run scope stands for both here.
+ var scope = data.Run;
+
+ // Detected targets are A z2, A z3, B, C, D - F fails q and is excluded from BOTH the
+ // denominator and the partner pool. If F leaked in it would outscore A (20.0 vs 9.0)
+ // and give A a better-scoring partner, so NBetter would be 2.
+ Assert.AreEqual(5, scope.Target.N);
+ Assert.AreEqual(2, scope.Target.NShared); // A (partner B) and B (partner A)
+ Assert.AreEqual(1, scope.Target.NBetter); // only B is outscored by its partner
+ Assert.AreEqual(0.2, scope.Target.BetterFraction, 1e-12);
+
+ // Entrapment and decoys are false by construction, so NBetter is how much of each
+ // would disappear under a best-match-wins rule on doubly-claimed peaks: all of it
+ // here, against a 0.2 target base rate.
+ Assert.IsNotNull(scope.Entrapment);
+ Assert.AreEqual(1, scope.Entrapment.N);
+ Assert.AreEqual(1, scope.Entrapment.NBetter);
+
+ Assert.IsNotNull(scope.Decoy);
+ Assert.AreEqual(1, scope.Decoy.N);
+ Assert.AreEqual(1, scope.Decoy.NBetter);
+
+ // THE DECOY BOUNDARY IS SCORE-SPACE, AND THE TWO SCOPES USE DIFFERENT SCORES.
+ // Decoys have no meaningful q of their own, so they are the one class admitted by
+ // comparing a score against the worst accepted target/entrapment. At RUN scope that
+ // comparison is the row's own Score; at EXPERIMENT scope it MUST be
+ // ExperimentAggregateScore, the score the experiment-wide competition actually
+ // ranked on. Decoy X is built to separate the two: score 6.0 clears file1's run
+ // boundary of 3.0, aggregate 1.0 does not clear the experiment boundary of 2.0.
+ //
+ // So a build that reads Score at experiment scope admits X and this assertion fails.
+ // Without it the entire v4 field is untested: every fixture row used to carry the
+ // 0.0 default, which made the experiment boundary 0.0 and admitted every decoy no
+ // matter what the code did. That is how the pass-2 panel shipped reporting 542,368
+ // decoys against 117,783 targets on astral.
+ Assert.IsNull(data.Experiment.Decoy,
+ @"a decoy whose experiment aggregate is below the experiment boundary must not be admitted");
+ // The classes gated on their own q are unaffected, which localizes any failure above
+ // to the decoy rule rather than to the acceptance set.
+ Assert.IsNotNull(data.Experiment.Target);
+ Assert.IsNotNull(data.Experiment.Entrapment);
+ Assert.AreEqual(1, data.Experiment.Entrapment.N);
+
+ // Every class here is far under MIN_N_FOR_ENRICHMENT, so the ratios are suppressed.
+ // A measured Stellar run accepted 7 decoys at experiment q and 1 of 7 rendered as
+ // "6.4x", indistinguishable at a glance from the 5.7x that took a 40-file cohort to
+ // establish. 1-of-1 here would read as an even more alarming 5x.
+ Assert.IsTrue(double.IsNaN(scope.Enrichment));
+ Assert.IsTrue(double.IsNaN(scope.DecoyEnrichment));
+
+ // None of these pairs is a PTM positional isomer - the sequences differ outright - so
+ // the whole "would go away" count survives the caveat subtraction.
+ Assert.AreEqual(0, scope.Entrapment.NBetterSameBaseSequence);
+ Assert.AreEqual(0, scope.Target.NBetterSameBaseSequence);
+ Assert.AreEqual(@"PEPTIDE",
+ ModelDiagnosticsData.CoAssignmentAccumulator.StripModifications(@"PEPT[+79.966]IDE"));
+
+ // The tolerance ladder falls out of the retained per-precursor minima: B is co-assigned
+ // at 0.018 min and D at 0.202, so the target rate steps 0 -> 1/5 at 0.02 and 1/5 -> 2/5
+ // at 0.25. This is the sensitivity the issue insists on showing rather than baking in.
+ CollectionAssert.AreEqual(new[] { 0.01, 0.02, 0.05, 0.10, 0.25 }, data.ToleranceLadder);
+ Assert.AreEqual(0.0, scope.Target.BetterByTolerance[0], 1e-12);
+ Assert.AreEqual(0.2, scope.Target.BetterByTolerance[1], 1e-12);
+ Assert.AreEqual(0.2, scope.Target.BetterByTolerance[3], 1e-12);
+ Assert.AreEqual(0.4, scope.Target.BetterByTolerance[4], 1e-12);
+
+ // MATCHING IS ON PRECURSOR m/z, NOT NEUTRAL MASS. "A" at z2 and z3 have identical
+ // neutral mass and identical apex RT, so a neutral-mass test would pair them - which
+ // is the only reason the prototype needed a same-sequence exclusion. Under m/z they
+ // are 500.000 vs 333.670 and cannot pair, so the z3 row contributes no co-assignment
+ // and the ladder's first entry stays 0 (a neutral-mass regression makes it 1/5).
+ Assert.AreEqual(0.0, scope.Target.BetterByTolerance[0], 1e-12);
+
+ // Pre-compaction dedup: A's second row in file1 (score 2.0, apex 10.150) must lose to
+ // its best-scoring row, so A's nearest partner is B at 0.018 min. Had the duplicate
+ // won, the histogram would carry 0.132 instead.
+ int binWidth200 = 200; // 50 bins over 0.25 min
+ Assert.AreEqual(2, scope.DeltaRtTarget[(int)(0.018 * binWidth200)]); // A<->B, both directions
+ Assert.AreEqual(2, scope.DeltaRtTarget[(int)(0.202 * binWidth200)]); // C<->D, both directions
+ Assert.IsNotNull(scope.DeltaRtEntrapment);
+ Assert.AreEqual(1, scope.DeltaRtEntrapment[(int)(0.032 * binWidth200)]);
+
+ // Runs are scanned independently: A is in both files, and its file2 peak at 30.0 min
+ // has no partner. Nothing pairs across runs, which would not be a shared peak at all.
+ Assert.AreEqual(3, scope.WorstOffenders.Count);
+
+ // KNOWN-FALSE CLASSES LEAD, then score gap. Entrapment E (gap 5.0) outranks decoy X
+ // (gap 3.0) and target B (gap 6.0) despite the smaller gap, because entrapment is
+ // absent by construction and is therefore the only DEMONSTRATED error. Ranking on gap
+ // alone put zero entrapment rows in a real 50-row listing - targets outnumber
+ // entrapment ~75:1 - so the class priority decides what reaches the report at all.
+ Assert.AreEqual(@"E", scope.WorstOffenders[0].ModifiedSequence);
+ Assert.AreEqual(@"A", scope.WorstOffenders[0].PartnerModifiedSequence);
+ Assert.AreEqual(@"file1", scope.WorstOffenders[0].File);
+ CollectionAssert.AreEqual(new[] { @"PTarget", @"Decoy", @"Target" },
+ scope.WorstOffenders.ConvertAll(o => o.Class));
+ // Target B has the LARGEST gap (6.0) and still sorts last, behind decoy X (3.0):
+ // class priority outranks the gap, which is the whole point.
+ Assert.AreEqual(6.0, scope.WorstOffenders[2].ScoreGap, 1e-12);
+
+ // DECOYS ARE INCLUDED BY SCORE, NOT BY THEIR OWN q. A decoy's q is a byproduct of the
+ // competition decoys themselves define, so gating on it asks the ruler to grade
+ // itself. The boundary is the worst-scoring accepted target/entrapment precursor in
+ // the run (B at 3.0 in file1); X at 6.0 clears it. Drop X below that and the class
+ // must empty out entirely.
+ var lowDecoy = new List(f1);
+ lowDecoy[5] = CoEntry(5 | DECOY_BIT, true, 1.0, 0.004, @"X", 2, 10.008);
+ var below = ModelDiagnosticsData.BuildCoAssignment(
+ WrapFiles(lowDecoy, f2), cls, id => mz.TryGetValue(id, out double v) ? v : double.NaN,
+ 0.01, FdrLevel.Precursor, 1, false);
+ Assert.IsNull(below.Run.Decoy);
+
+ // No resolvable library m/z means the panel cannot be computed at all, and must say so
+ // by returning null rather than reporting a zero co-assignment rate.
+ Assert.IsNull(ModelDiagnosticsData.BuildCoAssignment(
+ WrapFiles(f1, f2), cls, id => double.NaN, 0.01, FdrLevel.Precursor, 1, false));
+
+ TestCoAssignmentEnrichmentAndOffenderDedup();
+ TestCoAssignmentAggregateStubDoesNotOutrankRealScore();
+ TestCoAssignmentExactTieGoesToTheDecoy();
+ }
+
+ // A row still carrying the ResetScores 0.0 default must NOT outrank the entry's real
+ // experiment aggregate. The reduction in ObserveCutoff used to be a plain max(), defended
+ // on the grounds that a stub must not pull a real aggregate DOWN to zero - which only
+ // holds if aggregates are mostly positive. Measured on the 34-file SEA-AD 2nd-pass
+ // sidecars they are overwhelmingly negative (93.2%, boundary at -2.33), so 0.0 is an
+ // extreme upper outlier and max() hands the stub the win every time it appears.
+ //
+ // The fixture is built so the two rules give opposite answers:
+ //
+ // target A accepted, aggregate -1.0
+ // target B accepted, aggregate -5.0 <- the worst accepted, so it sets the boundary
+ // target B a SECOND row for the same entry, aggregate 0.0 (the stub)
+ // decoy X aggregate -4.0
+ //
+ // prefer-real: B stays -5.0, boundary -5.0, decoy -4.0 clears it and IS admitted.
+ // max(): B becomes 0.0, boundary -1.0, decoy -4.0 misses it and vanishes.
+ //
+ // So reverting to max() turns the decoy row null and fails the assertion below. Note the
+ // direction: here the stub SUPPRESSES a real decoy, where the astral defect inflated the
+ // count. Both are the same collapse toward 0.0, and which way it lands depends only on
+ // whether the stub sits on an accepted target or on a decoy.
+ private static void TestCoAssignmentAggregateStubDoesNotOutrankRealScore()
+ {
+ var mz = new Dictionary
+ {
+ { 1, 500.000 }, { 2, 500.004 }, { 5 | DECOY_BIT, 500.006 },
+ };
+ var cls = new Dictionary
+ {
+ { 1, EntrapmentClass.Target }, { 2, EntrapmentClass.Target },
+ };
+ var file = new List
+ {
+ CoEntry(1, false, 9.0, 0.001, "A", 2, 10.000, -1.0),
+ CoEntry(2, false, 3.0, 0.002, "B", 2, 10.018, -5.0),
+ // The stub: same entry as the row above, left at the ResetScores default.
+ CoEntry(2, false, 0.0, 0.002, "B", 2, 10.150, 0.0),
+ CoEntry(5 | DECOY_BIT, true, 6.0, 0.004, "X", 2, 10.008, -4.0),
+ };
+
+ var data = ModelDiagnosticsData.BuildCoAssignment(
+ WrapFiles(file), cls, id => mz.TryGetValue(id, out double v) ? v : double.NaN,
+ 0.01, FdrLevel.Precursor, 1, false);
+ Assert.IsNotNull(data);
+
+ Assert.IsNotNull(data.Experiment.Decoy,
+ @"the 0.0 stub outranked entry B's real -5.0 aggregate, lifting the experiment boundary and dropping a decoy that clears it");
+ Assert.AreEqual(1, data.Experiment.Decoy.N);
+
+ // The q-gated classes are untouched either way, which localizes a failure above to
+ // the aggregate reduction rather than to the acceptance set.
+ Assert.IsNotNull(data.Experiment.Target);
+ Assert.AreEqual(2, data.Experiment.Target.N);
+ }
+
+ // An EXACT target/decoy tie goes to the DECOY, matching what the competition that
+ // produced the q-values actually does: StreamingFdr computes
+ // `decoyWins = hasT && hasD ? !(t.score > d.score) : !hasT`, so a tied decoy is inside
+ // the FDR estimate that set the acceptance boundary. The panel used to require
+ // `decoyBest > targetBest` (and `tgt >= kv.Value` at run scope), which excluded exactly
+ // those decoys from the row the boundary is meant to admit.
+ //
+ // The fixture is built so the two rules give opposite answers:
+ //
+ // target A accepted, aggregate -1.0 <- the only accepted target
+ // decoy X aggregate -1.0, tied with its own target 5
+ // target 5 aggregate -1.0 <- NOT accepted (q = 0.5)
+ //
+ // decoy-wins-ties: X won its pair, clears the -1.0 boundary, and IS admitted.
+ // target-wins-ties: X "lost", is excluded, and the decoy class comes back null.
+ private static void TestCoAssignmentExactTieGoesToTheDecoy()
+ {
+ var mz = new Dictionary
+ {
+ { 1, 500.000 }, { 5, 500.006 }, { 5 | DECOY_BIT, 500.006 },
+ };
+ var cls = new Dictionary
+ {
+ { 1, EntrapmentClass.Target }, { 5, EntrapmentClass.Target },
+ };
+ var file = new List
+ {
+ CoEntry(1, false, 9.0, 0.001, @"A", 2, 10.000, -1.0),
+ // Rejected, so it does not move the boundary - it exists only to be the decoy's
+ // tied competitor.
+ CoEntry(5, false, 4.0, 0.500, @"T5", 2, 10.004, -1.0),
+ CoEntry(5 | DECOY_BIT, true, 4.0, 0.500, @"T5", 2, 10.008, -1.0),
+ };
+
+ var data = ModelDiagnosticsData.BuildCoAssignment(
+ WrapFiles(file), cls, id => mz.TryGetValue(id, out double v) ? v : double.NaN,
+ 0.01, FdrLevel.Precursor, 1, false);
+ Assert.IsNotNull(data);
+ Assert.IsNotNull(data.Experiment.Decoy,
+ @"an exact target/decoy tie was resolved against the decoy, dropping it from the row the boundary admits - StreamingFdr gives the tie to the decoy");
+ Assert.AreEqual(1, data.Experiment.Decoy.N);
+ }
+
+ // Enrichment arithmetic above MIN_N_FOR_ENRICHMENT, and one offender ROW per precursor
+ // pair rather than one per observation. Both need a bigger pool than the fixture above:
+ // 31 targets and 31 entrapment, padded with isolated precursors at m/z nobody shares.
+ // 1 of 31 targets and 2 of 31 entrapment are co-assigned, so enrichment is exactly 2.0.
+ private static void TestCoAssignmentEnrichmentAndOffenderDedup()
+ {
+ var mz = new Dictionary { { 1, 500.000 }, { 2, 500.004 }, { 201, 500.005 }, { 202, 500.006 } };
+ var cls = new Dictionary
+ {
+ { 1, EntrapmentClass.Target }, { 2, EntrapmentClass.Target },
+ { 201, EntrapmentClass.PTarget }, { 202, EntrapmentClass.PTarget },
+ };
+ var rows = new List
+ {
+ CoEntry(1, false, 9.0, 0.001, "A", 2, 10.000), // the strong explanation
+ CoEntry(2, false, 3.0, 0.001, "B", 2, 10.018), // co-assigned target
+ CoEntry(201, false, 4.0, 0.001, "E1", 2, 10.020), // co-assigned entrapment
+ CoEntry(202, false, 4.0, 0.001, "E2", 2, 10.022), // co-assigned entrapment
+ };
+ for (uint i = 0; i < 29; i++) // isolated padding, no partners
+ {
+ uint t = 1000 + i, p = 2000 + i;
+ mz[t] = 600.0 + i; mz[p] = 800.0 + i;
+ cls[t] = EntrapmentClass.Target; cls[p] = EntrapmentClass.PTarget;
+ rows.Add(CoEntry(t, false, 5.0, 0.001, "T" + i, 2, 20.0 + i));
+ rows.Add(CoEntry(p, false, 5.0, 0.001, "P" + i, 2, 40.0 + i));
+ }
+ // The same two runs, so every co-assigned pair is seen TWICE. Row count per pair must
+ // still be one, with Runs == 2.
+ var data = ModelDiagnosticsData.BuildCoAssignment(
+ WrapFiles(rows, new List(rows)), cls,
+ id => mz.TryGetValue(id, out double v) ? v : double.NaN,
+ 0.01, FdrLevel.Precursor, 1, false);
+ Assert.IsNotNull(data);
+ var scope = data.Run;
+ Assert.AreEqual(31, scope.Target.N);
+ Assert.AreEqual(1, scope.Target.NBetter); // B, outscored by A
+ Assert.AreEqual(31, scope.Entrapment.N);
+ Assert.AreEqual(2, scope.Entrapment.NBetter); // E1 and E2, both outscored by A
+ Assert.AreEqual(2.0, scope.Enrichment, 1e-12);
+
+ // Three distinct pairs, each observed in both runs: 3 rows, not 6.
+ Assert.AreEqual(3, scope.WorstOffenders.Count);
+ foreach (var o in scope.WorstOffenders)
+ Assert.AreEqual(2, o.Runs);
}
// The streaming pass-1 accumulator (fed per-row off the projection score-pass sink so an
@@ -1138,6 +1467,31 @@ private static FdrEntry Entry(uint id, bool decoy, double score, double q, strin
};
}
+ // An entry carrying a detection apex RT, for the peak co-assignment panel (the only card
+ // that reads FdrEntry.ApexRt).
+ ///
+ /// A co-assignment fixture row. is the EXPERIMENT AGGREGATE
+ /// SCORE (sidecar v4), which defaults to the row's own score - right for an entry with a
+ /// single observation, and the reason it can be omitted on rows where the distinction
+ /// does not matter.
+ ///
+ /// It must be settable, and it must default to something other than 0.0. The
+ /// experiment-scope decoy boundary is the ONLY quantity on this panel gated by score
+ /// rather than by a q, and it reads this field, not Score. Leaving every fixture
+ /// row at the ResetScores 0.0 default made the boundary 0.0, admitted every decoy
+ /// unconditionally, and let a real defect ship green - the pass-2 panel reported 542,368
+ /// decoys against 117,783 targets on astral before it was caught by inspecting the
+ /// rebaselined golden rather than by this test.
+ ///
+ private static FdrEntry CoEntry(uint id, bool decoy, double score, double q,
+ string seq, byte charge, double apexRt, double expAgg = double.NaN)
+ {
+ var entry = Entry(id, decoy, score, q, seq, charge);
+ entry.ApexRt = apexRt;
+ entry.ExperimentAggregateScore = double.IsNaN(expAgg) ? score : expAgg;
+ return entry;
+ }
+
// An entry with distinct per-run and experiment-wide precursor q (for the
// per-scope yield curve, where the default Entry sets both scopes equal).
private static FdrEntry EntryQ(uint id, bool decoy, double score,
diff --git a/pwiz_tools/Osprey/Osprey.Test/Pass2FdrSidecarTest.cs b/pwiz_tools/Osprey/Osprey.Test/Pass2FdrSidecarTest.cs
index cf2feb5e0f..0e4833e5ce 100644
--- a/pwiz_tools/Osprey/Osprey.Test/Pass2FdrSidecarTest.cs
+++ b/pwiz_tools/Osprey/Osprey.Test/Pass2FdrSidecarTest.cs
@@ -350,17 +350,23 @@ public void TestAssignPerRunQCarriesExperimentQ()
// A well-identified 1st-pass record for a precursor: high score, low q at every level.
// rec.Score is the averaged-model score the pass-2 recomputation reproduces bit-exact.
+ // experimentAggregateScore is deliberately DIFFERENT from score: it is the
+ // cross-run roll-up, not the per-row discriminant, so a carry that confused the
+ // two would show up here.
var rec = new FdrScoreRecord(
entryId: 1, score: 10.0,
runPrecursorQvalue: 0.001, runPeptideQvalue: 0.002,
experimentPrecursorQvalue: 0.0005, experimentPeptideQvalue: 0.0006,
- pep: 0.03, runProteinQvalue: 0.004);
+ pep: 0.03, runProteinQvalue: 0.004, experimentAggregateScore: 12.5);
// (a) UNCHANGED: recomputed score == the record's score -> carry the whole record.
var unchanged = new FdrEntry { EntryId = 1 };
var clsU = Pass2FdrSidecar.AssignPerRunQ(unchanged, 10.0, rec,
- precScoresDesc, precQDesc, pepScoresDesc, pepQDesc, 1.0, 1.0);
+ precScoresDesc, precQDesc, pepScoresDesc, pepQDesc, 1.0, 1.0, 0.0);
Assert.AreEqual(Pass2FdrSidecar.PerRunClass.Unchanged, clsU);
+ // From the RECORD, not the gap-fill argument: an Unchanged peak has a record, so the
+ // 0.0 passed above must be ignored.
+ Assert.AreEqual(12.5, unchanged.ExperimentAggregateScore, 1e-12);
Assert.AreEqual(10.0, unchanged.Score, 1e-12);
Assert.AreEqual(0.001, unchanged.RunPrecursorQvalue, 1e-12);
Assert.AreEqual(0.002, unchanged.RunPeptideQvalue, 1e-12);
@@ -373,7 +379,7 @@ public void TestAssignPerRunQCarriesExperimentQ()
// invariant: only per-run q moves, and only toward higher (less confident) values.
var moved = new FdrEntry { EntryId = 1 };
var clsM = Pass2FdrSidecar.AssignPerRunQ(moved, 5.0, rec,
- precScoresDesc, precQDesc, pepScoresDesc, pepQDesc, 1.0, 1.0);
+ precScoresDesc, precQDesc, pepScoresDesc, pepQDesc, 1.0, 1.0, 0.0);
Assert.AreEqual(Pass2FdrSidecar.PerRunClass.Moved, clsM);
Assert.AreEqual(5.0, moved.Score, 1e-12);
Assert.AreEqual(0.01, moved.RunPrecursorQvalue, 1e-12); // table lookup at score 5
@@ -385,15 +391,21 @@ public void TestAssignPerRunQCarriesExperimentQ()
// (c) GAP-FILL: no 1st-pass record -> run q from the table; experiment q takes the
// precursor's supplied cross-file pass-1 value (the clamp later floors it correctly).
+ // The experiment aggregate score comes from that SAME cross-file source: a gap-fill
+ // that took the q without the score would persist a real q beside ResetScores' 0.0,
+ // and a score-space acceptance boundary read back from the 2nd-pass sidecar would
+ // then collapse onto that zero.
var gap = new FdrEntry { EntryId = 2 };
+ gap.ResetScores();
var clsG = Pass2FdrSidecar.AssignPerRunQ(gap, 5.0, null,
- precScoresDesc, precQDesc, pepScoresDesc, pepQDesc, 0.004, 0.006);
+ precScoresDesc, precQDesc, pepScoresDesc, pepQDesc, 0.004, 0.006, 7.25);
Assert.AreEqual(Pass2FdrSidecar.PerRunClass.GapFill, clsG);
Assert.AreEqual(5.0, gap.Score, 1e-12);
Assert.AreEqual(0.01, gap.RunPrecursorQvalue, 1e-12);
Assert.AreEqual(0.02, gap.RunPeptideQvalue, 1e-12);
Assert.AreEqual(0.004, gap.ExperimentPrecursorQvalue, 1e-12);
Assert.AreEqual(0.006, gap.ExperimentPeptideQvalue, 1e-12);
+ Assert.AreEqual(7.25, gap.ExperimentAggregateScore, 1e-12);
}
// Verbatim copy of the FdrProjectionSet-overload comparer in
diff --git a/pwiz_tools/Osprey/Osprey.Test/TaskValidityKeyTest.cs b/pwiz_tools/Osprey/Osprey.Test/TaskValidityKeyTest.cs
index a340847aa9..a3365d1bdb 100644
--- a/pwiz_tools/Osprey/Osprey.Test/TaskValidityKeyTest.cs
+++ b/pwiz_tools/Osprey/Osprey.Test/TaskValidityKeyTest.cs
@@ -21,9 +21,11 @@
* limitations under the License.
*/
+using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using pwiz.Osprey.Core;
using pwiz.Osprey.Tasks;
+using pwiz.Osprey.Tasks.ModelDiagnostics;
namespace pwiz.Osprey.Test
{
@@ -49,6 +51,54 @@ public void TestFlippedDefaultsParticipateInTheValidityKey()
AssertEachArmKeysDifferently();
AssertEveryTaskCarriesTheSuffixesItNeeds();
AssertLibraryFragmentArmIsPinnedToThePipeline();
+ AssertDiagnosticsReportIsADeclaredOutputOnlyWhenAsked();
+ }
+
+ ///
+ /// The --model-diagnostics report must be a DECLARED OUTPUT of the task that
+ /// finalizes it, and only when the flag is on.
+ ///
+ /// Declared, because that is the whole mechanism by which a completed run can
+ /// regenerate a deleted report: task validity requires every declared output to exist,
+ /// so a missing HTML invalidates SecondPassFDR alone, Stages 1-5 stay cached, and the
+ /// pass-1 panel is rebuilt by rehydrating the 1st-pass sidecars. Before this the flag
+ /// was INERT on a cached directory - it is in no validity key, the HTML was in no
+ /// Outputs list, so re-running with it added skipped every task and produced no
+ /// report at all.
+ ///
+ /// Only when asked, because declaring it unconditionally would leave every run
+ /// that never wanted diagnostics permanently invalid, re-running SecondPassFDR on
+ /// every resume forever.
+ ///
+ private static void AssertDiagnosticsReportIsADeclaredOutputOnlyWhenAsked()
+ {
+ foreach (bool wanted in new[] { false, true })
+ {
+ var config = new OspreyConfig
+ {
+ OutputBlib = @"C:\runs\out.blib",
+ ModelDiagnostics = wanted
+ };
+ var tasks = AnalysisPipeline.CanonicalPipeline();
+ var ctx = new PipelineContext(config, tasks, null, null, null);
+ OspreyTask second = null;
+ foreach (var t in tasks)
+ {
+ if (t.Name == @"SecondPassFDR")
+ second = t;
+ }
+ Assert.IsNotNull(second, @"SecondPassFDR must be in the canonical pipeline");
+
+ bool declared = false;
+ foreach (string o in second.Outputs(ctx))
+ {
+ if (o != null && o.EndsWith(ModelDiagnosticsReport.HtmlSuffix, StringComparison.Ordinal))
+ declared = true;
+ }
+ Assert.AreEqual(wanted, declared, wanted
+ ? @"the report must be a declared output when --model-diagnostics is on, or a deleted report cannot be regenerated"
+ : @"the report must NOT be declared when --model-diagnostics is off, or every plain run is permanently invalid");
+ }
}
///
diff --git a/pwiz_tools/Osprey/Osprey/OspreyCommandArgs.cs b/pwiz_tools/Osprey/Osprey/OspreyCommandArgs.cs
index c5035bdabc..cd20c6c49f 100644
--- a/pwiz_tools/Osprey/Osprey/OspreyCommandArgs.cs
+++ b/pwiz_tools/Osprey/Osprey/OspreyCommandArgs.cs
@@ -287,6 +287,13 @@ static OspreyCommandArgs()
// format/section value (ascii | unicode | sections | html | ).
public static readonly OspreyArgument ARG_DIAGNOSTICS = new OspreyArgument(@"diagnostics",
(c, p) => c._config.Diagnostics = true) { ShortName = @"d" };
+ // One flag, everything we know how to show. An opt-in token per expensive panel was built
+ // and removed (#4522): the peak co-assignment panel measured 7.3M rows/s, i.e. ~46s on an
+ // 82-file Astral run against a 10-hour search, so the cost never justified making anyone
+ // choose. Someone who asks for --model-diagnostics wants the diagnostics, not a decision
+ // about which ones they can afford - and a panel behind a token nobody remembers is a
+ // panel nobody sees, which defeats a diagnostic whose whole purpose is surfacing an effect
+ // users do not know to look for.
public static readonly OspreyArgument ARG_MODEL_DIAGNOSTICS = new OspreyArgument(@"model-diagnostics",
(c, p) => c._config.ModelDiagnostics = true);
public static readonly OspreyArgument ARG_HELP = new OspreyArgument(@"help",
@@ -420,7 +427,6 @@ private void TokenizeAndDispatch(string[] args)
matched.ProcessValue(this, new NameValuePair(matched.Name, parallelValue));
continue;
}
-
if (matched.Variadic)
{
i++;
@@ -809,7 +815,7 @@ private class OspreyArgUsageProvider : IArgUsageProvider
{ @"perf-stats", @"Emit machine-parseable [COUNT]/[TIMING]/[STAGE-WALL] lines for perf tools (off by default)" },
{ @"verbose", @"Show implementer-grade detail (e.g. per-fold Percolator iterations) hidden by default" },
{ @"diagnostics", @"Write cross-impl bisection dumps (OSPREY_DUMP_* bundle)" },
- { @"model-diagnostics", @"Write a self-contained interactive HTML report of the trained scoring model and FDR calibration" },
+ { @"model-diagnostics", @"Write a self-contained interactive HTML report of the trained scoring model, FDR calibration, and single-peak multiple-ID co-assignment" },
{ @"help", @"Show this help message ([ascii|unicode|sections|html|])" },
{ @"version", @"Show version" },
};
diff --git a/pwiz_tools/Osprey/Regression/DiagnosticsGolden.ps1 b/pwiz_tools/Osprey/Regression/DiagnosticsGolden.ps1
index 5b33a0cd8d..cc6ce7d0bc 100644
--- a/pwiz_tools/Osprey/Regression/DiagnosticsGolden.ps1
+++ b/pwiz_tools/Osprey/Regression/DiagnosticsGolden.ps1
@@ -162,6 +162,37 @@ function Get-DiagnosticsMetrics {
Add-Metric 'winFraction.nullBandReal' $d.winFraction.nullBandReal
Add-Metric 'winFraction.nullBandEnt' $(if ($d.winFraction.hasEntrapment) { $d.winFraction.nullBandEnt } else { $null })
+ # --- Peak co-assignment (issue #4522) ----------------------------------
+ # Pinned at BOTH passes and BOTH q scopes. Without these the panel ships with no golden
+ # coverage at all: this projection is an explicit metric list, not an enumeration of the
+ # payload, so a new card is invisible to the comparison until it is named here. The counts
+ # are the critical ones (nBetter is the "would go away under best-match-wins" number);
+ # the fractions follow from them and n, so pinning both would only double the failure noise.
+ foreach ($p in 1, 2) {
+ $ca = if ($p -eq 2) { $d.pass2.coAssignment } else { $d.coAssignment }
+ foreach ($scope in 'run', 'experiment') {
+ $s = if ($ca) { $ca.$scope } else { $null }
+ foreach ($cls in 'target', 'entrapment', 'decoy') {
+ $r = if ($s) { $s.$cls } else { $null }
+ Add-Metric "pass$p.coAssign.$scope.$cls.n" $(if ($r) { $r.n } else { $null })
+ Add-Metric "pass$p.coAssign.$scope.$cls.nBetter" $(if ($r) { $r.nBetter } else { $null })
+ }
+ # NaN when a class is under MIN_N_FOR_ENRICHMENT, which is itself worth pinning: it
+ # says the run had too few of that class to make a ratio, and a change in that is a
+ # change in the pool.
+ Add-Metric "pass$p.coAssign.$scope.enrichment" $(if ($s) { $s.enrichment } else { $null })
+ }
+ # The acceptance boundary in use, and the score this pass's OWN population needs to reach
+ # the target FDR. Pinned because their divergence is the pool-selection signal: they agree
+ # at pass 1 and separate at pass 2 when compaction has stripped the winning decoys, and a
+ # change in that separation is a change in how the second pass is built. The counts are
+ # pinned beside the score so a drift shows whether it moved the IDs, the decoys, or both.
+ Add-Metric "pass$p.coAssign.cutoff" $(if ($ca) { $ca.experimentCutoff } else { $null })
+ Add-Metric "pass$p.coAssign.fdrCrossing" $(if ($ca) { $ca.experimentFdrCrossing } else { $null })
+ Add-Metric "pass$p.coAssign.fdrCrossingDecoys" $(if ($ca) { $ca.experimentFdrCrossingDecoys } else { $null })
+ Add-Metric "pass$p.coAssign.fdrCrossingNonDecoys" $(if ($ca) { $ca.experimentFdrCrossingNonDecoys } else { $null })
+ }
+
# --- FDP at the reported-q threshold (entrapment only) -----------------
foreach ($pass in 1, 2) {
$fdp = Get-FdpAtThreshold -Payload $d -Pass $pass -Scope 'experiment'
@@ -261,9 +292,22 @@ function Compare-DiagnosticsGolden {
$bothNumeric = [double]::TryParse($g, $style, $inv, [ref]$gd) -and
[double]::TryParse($f, $style, $inv, [ref]$fd)
if ($bothNumeric) {
- $diff = [math]::Abs($gd - $fd)
- if ($diff -gt $Tolerance) {
- $issues.Add(("diagnostics: {0} golden={1} run={2} diff={3:e3} (tol {4:e0})" -f $name, $g, $f, $diff, $Tolerance))
+ # NaN and +/-Infinity BOTH parse successfully here, and every comparison against
+ # NaN is $false - so a bare Abs(diff) -gt Tolerance passes golden='NaN' against
+ # run='4.37' silently, and the string fallback below is unreachable once TryParse
+ # has succeeded. NaN is a meaningful VALUE for these metrics (a class under
+ # MIN_N_FOR_ENRICHMENT reports it), so a class crossing that threshold in either
+ # direction is exactly the change worth catching. Compare non-finite values for
+ # equality; only finite pairs get the tolerance.
+ if (-not ([double]::IsFinite($gd) -and [double]::IsFinite($fd))) {
+ if ($g -ne $f) {
+ $issues.Add(("diagnostics: {0} golden='{1}' run='{2}'" -f $name, $g, $f))
+ }
+ } else {
+ $diff = [math]::Abs($gd - $fd)
+ if ($diff -gt $Tolerance) {
+ $issues.Add(("diagnostics: {0} golden={1} run={2} diff={3:e3} (tol {4:e0})" -f $name, $g, $f, $diff, $Tolerance))
+ }
}
} elseif ($g -ne $f) {
$issues.Add(("diagnostics: {0} golden='{1}' run='{2}'" -f $name, $g, $f))
diff --git a/pwiz_tools/Osprey/Regression/FdrSidecars.ps1 b/pwiz_tools/Osprey/Regression/FdrSidecars.ps1
index 409b730fb2..d62b251081 100644
--- a/pwiz_tools/Osprey/Regression/FdrSidecars.ps1
+++ b/pwiz_tools/Osprey/Regression/FdrSidecars.ps1
@@ -19,9 +19,10 @@ they agreed on the wrong value and nothing was red.
This compares the sidecars themselves, which is where the distributed tasks' per-file
output actually lands.
-Record layout (Osprey.IO\FdrScoresSidecar.cs): 32-byte header, 60-byte records,
+Record layout (Osprey.IO\FdrScoresSidecar.cs), v4: 32-byte header, 68-byte records,
entry_id u32 @0, score f64 @4, run_precursor_q @12, run_peptide_q @20,
- experiment_precursor_q @28, experiment_peptide_q @36, pep @44, run_protein_q @52.
+ experiment_precursor_q @28, experiment_peptide_q @36, pep @44, run_protein_q @52,
+ experiment_aggregate_score @60 (issue #4522).
Header: magic @0..8, version @8, pass @9, record count u64 @16.
The decode + compare runs as compiled C#, not PowerShell. A per-record PowerShell loop
@@ -67,8 +68,8 @@ public class FdrSidecarDiff
public static class OspreyFdrSidecarComparer
{
private const int HeaderLen = 32;
- private const int RecordLen = 60;
- private const byte ExpectedVersion = 3;
+ private const int RecordLen = 68;
+ private const byte ExpectedVersion = 4;
private static readonly byte[] Magic = { 0x4F, 0x53, 0x50, 0x52, 0x59, 0x46, 0x44, 0x52 }; // OSPRYFDR
/// Name and byte offset in ONE table. They were parallel arrays whose lengths separately
@@ -85,6 +86,7 @@ public static class OspreyFdrSidecarComparer
new FdrSidecarField { Name = "experiment_peptide_qvalue", Offset = 36 },
new FdrSidecarField { Name = "pep", Offset = 44 },
new FdrSidecarField { Name = "run_protein_qvalue", Offset = 52 },
+ new FdrSidecarField { Name = "experiment_aggregate_score", Offset = 60 },
};
public static FdrSidecarDiff Compare(
@@ -169,7 +171,12 @@ public static class OspreyFdrSidecarComparer
/// (FdrScoresSidecar.TryRead returns false on a pass mismatch) while a filename-only
/// comparison would happily call the two sides equal.
///
- /// The size arithmetic is checked: 60 divides many lengths, so a corrupt count can
+ /// The version check matters for the same reason: a writer whose record width differs from
+ /// ExpectedVersion's must be REFUSED by name rather than decoded at the wrong stride. The
+ /// size check alone rejected the v3 -> v4 growth only because 68 and 60 happen not to divide
+ /// alike, which is luck, not a guard.
+ ///
+ /// The size arithmetic is checked: 68 divides many lengths, so a corrupt count can
/// satisfy the size test by wrapping mod 2^64 and then walk off the end of the buffer.
/// The canonical reader wraps the identical expression for the identical reason.
private static byte[] ReadIfValid(string path, int expectedPass, out long count, out string problem)
diff --git a/pwiz_tools/Osprey/docs/14-intermediate-files.md b/pwiz_tools/Osprey/docs/14-intermediate-files.md
index 2058558cfc..b9fc671c43 100644
--- a/pwiz_tools/Osprey/docs/14-intermediate-files.md
+++ b/pwiz_tools/Osprey/docs/14-intermediate-files.md
@@ -18,8 +18,8 @@ resume mechanisms the port adds (a per-task `.osprey.task` validity sidecar and
| `.spectra.bin` | Custom binary v3 | `Osprey.IO/SpectraCache.cs` | Decoded MS1/MS2 spectra for fast reload |
| `.scores.parquet` | Apache Parquet (ZSTD) | `Osprey.IO/ParquetScoreCache.cs` | Scored entries: 21 PIN features, fragments, CWT candidates + footer metadata |
| `.scores-reconciled.parquet` | Apache Parquet (ZSTD) | `Osprey.Tasks/ReconciledParquetWriter.cs` | Stage 6 reconciled rewrite (separate file, not in-place) |
-| `.1st-pass.fdr_scores.bin` | Custom binary v3 | `Osprey.IO/FdrScoresSidecar.cs` | SVM score + 4 q-values + PEP + run_protein_qvalue after first-pass Percolator |
-| `.2nd-pass.fdr_scores.bin` | Custom binary v3 | `Osprey.IO/FdrScoresSidecar.cs` | Same record shape after second-pass Percolator |
+| `.1st-pass.fdr_scores.bin` | Custom binary v4 | `Osprey.IO/FdrScoresSidecar.cs` | SVM score + 4 q-values + PEP + run_protein_qvalue + experiment_aggregate_score after first-pass Percolator |
+| `.2nd-pass.fdr_scores.bin` | Custom binary v4 | `Osprey.IO/FdrScoresSidecar.cs` | Same record shape after second-pass Percolator |
| `.reconciliation.json` | JSON (Newtonsoft) | `Osprey.IO/ReconciliationFile.cs` | Stage 5 planner output: actions, gap-fill targets, refined RT calibration |
| `