OspreySharp: close C# Stage 7 algorithmic gap + port Rust v2 reconciliation envelope + parity-sprint perf work - #4233
Merged
brendanx67 merged 34 commits intoMay 27, 2026
Conversation
…elope * Wired 2nd-pass Percolator into MergeNodeTask; reloads PIN features from reconciled parquets before re-scoring on the rescored features. * Bumped ReconciliationFile to v2 with file_stems; added OspreyConfig.ReconciliationParameterHashForStems so workers stamp the join-wide hash that --join-at-pass=2 validates. * Hardened Stage 6 LoadMassCalibrations: throws on missing or unparseable calibration.json instead of silently degrading. * Folded in prior parity-sprint perf work (PercolatorFdr, SVM, scoring, scratch pools) and inspection cleanup (0 warnings, 346/346 tests). See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Companion PR to a Rust-side parity fix that closes the C# Stage 7 algorithmic gap discovered during performance work, ports the Rust v2 reconciliation.json envelope (with the file_stems field and join-wide hash), and folds in parity-sprint performance work (SVM grid-search parallelization via a new OspreyParallel, SvmTrainScratchPool, XCorr f32 scratch reuse, Percolator hot-path scratching). Also stops silently degrading Stage 6 when the Stage 1–4 calibration sidecar is missing, refactors RunPercolatorFdr to be reusable for the new 2nd-pass call in MergeNodeTask, and refreshes the workflow doc with new benchmarks.
Changes:
- Wires a missing 2nd-pass Percolator run into
MergeNodeTaskfor the HPC distribution case and refactorsFirstJoinTask.RunPercolatorFdrtointernal staticso it can be reused. - Adds v2
ReconciliationFile.FileStemsend-to-end (planner write, worker rehydration with cross-envelope consistency check,OspreyConfig.ReconciliationParameterHashForStems, worker stamps join-wide hash). - Performance work:
OspreyParallel.For+SvmTrainScratchPoolinLinearSvmClassifier/PercolatorFdr(parallel grid search, pooled extract / count-passing buffers), f32 scratch path inSpectralScorer.PreprocessSpectrumForXcorrInto, pooledMatrix.WrapPrefixNoClone.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pwiz_tools/OspreySharp/OspreySharp/Tasks/PerFileRescoreTask.cs | Threads joinFileStems through to WriteReconciledParquet; hard-errors on missing calibration sidecar. |
| pwiz_tools/OspreySharp/OspreySharp/Tasks/MergeNodeTask.cs | New 2nd-pass FDR path; reloads PIN features from reconciled parquets; renames timing tags to [STAGE-WALL]. |
| pwiz_tools/OspreySharp/OspreySharp/Tasks/FirstJoinTask.cs | Refactors RunPercolatorFdr to internal static with passLabel; populates file_stems in envelopes. |
| pwiz_tools/OspreySharp/OspreySharp/RescoreHydration.cs | Adds RescoreInputs.JoinFileStems; cross-envelope file_stems consistency check; normalize/equality helpers. |
| pwiz_tools/OspreySharp/OspreySharp/RescoreWorker.cs | Removes a now-unused using. |
| pwiz_tools/OspreySharp/OspreySharp/AnalysisPipeline.cs | Emits parseable [STAGE-WALL] lines per task. |
| pwiz_tools/OspreySharp/OspreySharp.IO/ReconciliationFile.cs | Bumps CurrentFormatVersion to 2; adds FileStems; reorders JSON ordinals. |
| pwiz_tools/OspreySharp/OspreySharp.IO/FileSaver.cs | Doc-only fix for ambiguous cref. |
| pwiz_tools/OspreySharp/OspreySharp.Core/OspreyConfig.cs | Adds ReconciliationParameterHashForStems; refactors original to call it. |
| pwiz_tools/OspreySharp/OspreySharp.FDR/PercolatorFdr.cs | Parallelizes grid search via OspreyParallel.For; scratch-pooled CountPassing/CompeteFromIndicesInto/ComputeQvaluesInto; ExtractRowsInto. |
| pwiz_tools/OspreySharp/OspreySharp.ML/LinearSvmClassifier.cs | New OspreyParallel, SvmTrainScratch, SvmTrainScratchPool; Train overload with scratch. |
| pwiz_tools/OspreySharp/OspreySharp.ML/Matrix.cs | New WrapPrefixNoClone; bounds AddInPlace/Divide by active size. |
| pwiz_tools/OspreySharp/OspreySharp.Scoring/SpectralScorer.cs | Actually uses scratch f32 buffers in PreprocessSpectrumForXcorrInto; extracts PreprocessSpectrumForXcorrF32IntoBuffers. |
| pwiz_tools/OspreySharp/OspreySharp.Scoring/XcorrScratchPool.cs | Adds f32 companion buffers (BinnedF/WindowedF/PrefixF). |
| pwiz_tools/OspreySharp/OspreySharp.Test/CalibrationTest.cs | Cross-OS path test fix using Path.Combine. |
| pwiz_tools/OspreySharp/OspreySharp.Test/CwtCandidateCodecTest.cs | WSL/Linux default test base dir fallback. |
| pwiz_tools/OspreySharp/Osprey-workflow.html | Updates benchmark tables and adds WSL perf section. |
* ParquetScoreCache.WriteScoresParquet now assigns entry.ParquetIndex = (uint)i, matching LoadFdrStubsFromParquet's convention. Without it, in-memory entries hit the Stage 5 reconciliation planner with ParquetIndex = 0 and every per-file CWT lookup returned the first row's candidates -- planner forced 35K entries on Stellar that the HPC path correctly use_cwt'd. * RescoreCompaction.Apply unions firstPassBaseIds with the base_ids of every entry the planner emits a reconciliation action for. The C# port of the Rust Option B fix landed this morning; without it the worker dropped ~200 cross-file-rescued entries per Stellar file with a warn-level log. * PerFileRescoreTask.Run short-circuits on ExpectReconciledInput (only running RescoreCompaction.Apply so MergeNodeTask sees the compacted set). Without it --join-at-pass=2 transitively triggered FirstJoinTask via EnsureHydrated, which re-ran Stage 5 + Stage 6 from scratch and tried to load mzML files the merge node does not have. Mirrors Rust pipeline.rs:3313-3344. * Updated TestRescoreCompactionRekeysActionsAndDropsNonpassing to TestRescoreCompactionUnionsActionsWithLocalFdrPredicate to assert the corrected union behavior. Verified by new C# in-memory vs C# HPC chain strict-rehydration test (Compare-Stage7-Rehydration-Strict-CSharp.ps1): Stages 5/6 sidecars + reconciled parquets byte-identical and Stage 7 blib + protein FDR dump match at 1e-9 on both Stellar 3-file and Astral 3-file. See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
* VERSION = "26.6.1" in Program.cs aligns the parquet footer metadata + blib OspreyMetadata with Rust upstream * TODO comment documents that the algorithmic substance of 26.6.1 (reconciliation pairing library-supplied decoys by base_id in compute_consensus_rts + plan_reconciliation) is not yet ported on the C# side. No effect on reverse-decoy mode (Stellar); will affect --decoys-in-library datasets See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
* .NET Framework F10 uses round-half-away-from-zero while Rust {:.10} uses round-half-to-even, producing 1-in-the-last-digit (1e-10) print diffs on f64s near a rounding boundary even when the underlying bits are bit-equal
* On Stellar 3-file 466K matches, apex_rt showed 15,739 rows "differing" by exactly ~1e-10 under F10; under F17 every matched row is bit-equal cross-impl
See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md
Co-Authored-By: Claude <noreply@anthropic.com>
* Same fix as commit 43100b1 applied to lda_scores dump * Closes the cross-impl 17-row "1e-10 drift" that was 100% F10-vs-:.10 formatter rounding artifact See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
Coordinated with Rust upstream osprey a44f752. The PreprocessSpectrumForXcorrInto cache-build path now runs the windowing and sliding-window cascade on the pooled XcorrScratch.Binned/Windowed/ Prefix double[] fields, narrowing to the per-spectrum float[] cache only at the final sliding-window store. The cache stays f32 to keep HRAM memory at 400 KB per spectrum, but values now carry single-cast precision rather than the f32-cascade noise the prior "imitate Rust f32" path produced. * PreprocessSpectrumForXcorrF32IntoBuffers: takes double[] binned / windowed / prefix and float[] preprocessed. Binning uses Math.Sqrt(float) (implicit widen-before-sqrt) so it bit-equals Rust's (intensity as f64).sqrt(). * ApplySlidingWindowDIntoF32: new helper, twin of ApplySlidingWindowD with an f32 result array. Final f64->f32 cast at store. * PreprocessSpectrumForXcorrInto: switches from BinnedF/WindowedF/ PrefixF (float[] companion fields) to the existing double[] scratch fields. Same pool, no new allocation. * XcorrScratch: removed the float[] BinnedF/WindowedF/PrefixF companion fields; no longer needed. * ApplyWindowingNormalizationF / ApplySlidingWindowF: removed. Cross-impl results on Stellar Single after the coordinated change: cal_match xcorr drift 3.876e-6 -> 4.429e-8, Stage 4 .scores.parquet xcorr at f32 single-cast architectural floor (max 5.41e-7 vs prior f32-cascade ~1e-6), rt_deviation max 2.32e-11 (LOESS cascade now sees bit-equal calibration inputs), peak_apex 100% bit-equal.
…sors
PerFileScoringTask wrote 0 for both metadata counts when serializing
the calibration JSON, leaving Rust as the only side with real values
in the file (Rust num_confident_peptides=7361, num_sampled_precursors=
192289 on Stellar Single). This populates both with the same semantics
Rust uses (osprey/src/pipeline.rs:1144-1145):
* NumConfidentPeptides = rtCalibration.Stats().NPoints (LOESS points
actually used by the fit that was saved; post S/N filter).
* NumSampledPrecursors = pass 1 matchArray.Length (total scored
matches, pre any q-value/S/N filter). Pass 2 is a refinement using
the narrowed RT tolerance and does not change the sampled count.
Plumbed via:
* CalibrationPassResult gains MatchCount field, set to matchArray.Length
inside RunCalibrationScoringPass.
* RunCalibration gains `out int numSampledPrecursors` returning pass 1's
MatchCount (only populated on the success path; 0 on load-from-cache
branch since no fresh scoring happens then).
* PerFileScoringTask plumbs the out param into the CalibrationMetadata
block.
Cross-impl effect on Stellar Single calibration.json comparison: max
absolute diff drops from 1.92e+5 (at num_sampled_precursors) to 1.75e+2
(at ms1_calibration.count), removing the metadata fields from the top
divergence list. Remaining content drift is ms1_calibration.{count,
mean, median, sd, adjusted_tolerance} - separate root cause (ms1_error
collection / mass calibration path divergence).
Mirrors osprey adfbea4. LINQ OrderBy is stable but the upstream input order can vary cross-impl when discriminant-score-sorted ties differ by 1 ULP. Sorting by (x, y) with ThenBy ensures duplicate-x positions get the same indices on both sides, eliminating cosmetic abs_residuals[i] swaps in calibration.json.
RTCalibration.cs:120 (outer wrapper around LoessRegression.Fit) sorted
by libraryRts ONLY. LoessRegression.cs:258 (inner) sorts by (x, y)
tuple. At duplicate-x positions the two sorts produced different
orders, so the outer y[i] and the inner fitted[i] ended up
corresponding to DIFFERENT data points:
outer: x[5]=1.93, y[5]=2.31320 (input-order tiebreak)
inner: sortedX[5]=1.93, sortedY[5]=2.30385, fitted[5] computed
from sortedY=2.30385
Then `residuals[i] = y[i] - fitted[i]` computed in the outer code at
line 173 became `residuals[5] = 2.31320 - 2.099 = 0.214` instead of
the correct `0.205`, with [5] and [6] swapping cross-impl. Rust is
consistent because it has only ONE sort (rt.rs:123 sorts by (x, y)
tuple in a single place, so y[i] and fitted[i] always correspond).
Fix: outer sort now also uses ThenBy on measuredRts so the two sorts
agree on duplicate-x order.
Cross-impl effect on Stellar Single calibration.json comparison:
max_diff drops 7.04e-1 -> 4.71e-12 (~11 orders of magnitude tighter).
All rt_calibration.model_params.abs_residuals[i] now at f64 epsilon
cross-impl. Remaining cal_json drift is f64 cascade noise in MAD /
abs_residuals from the bisquare-weighted refit (essentially bit-equal).
Two related fixes for cross-impl Stage 3 calibration parity: 1) DiannTsvLoader ParseDouble / ParseFloat / ParseDoubleOrDefault switch from double.TryParse to XmlConvert.ToDouble (and the float variant). XmlConvert is required by the XML schema spec to be IEEE-754 correct on round-trip, whereas .NET Framework 4.7.2's double.TryParse can be off by a few ULPs on 16-digit scientific values. Rust's str::parse::<f64> is IEEE-correct, so this brings the two parsers into bit-for-bit agreement on TSV library values like the "400.1277954005887" precursor m/z. 2) cal_scalars dump uses G17 instead of F17 for the f64 fields. .NET Framework 4.7.2's F17 format truncates double output at ~15 significant digits and pads to 17 fractional with zeros, so a round-trip "string -> parse -> f64" yields a different f64 than the original. G17 prints all 17 significant digits required for round-trip-safe representation. Without this fix, rt_min / rt_max / mz_min / mz_max / rt_range / mz_range / rt_bin_width / mz_bin_width all showed 1-5 ULP cross-impl diffs that were actually print-only artifacts. Cross-impl effect on Stellar Single: CAL_SAMPLE rust_cal_scalars.txt now PASS bit-equal (was FAIL 5.116e-13). Boundary count: 4 PASS / 5 FAIL -> 5 PASS / 4 FAIL. The XmlConvert fix is correctness-only on this dataset (the existing double.TryParse produced the same f64 we have today). It guards against future TSV inputs that would trigger the .NET Framework parser's 1-2 ULP corner cases.
Same .NET Framework 4.7.2 print-precision issue we hit in cal_scalars (commit 11943b1): F17 truncates double output at ~15 significant digits and pads the rest with zeros, so a string -> parse round-trip yields a different f64 than the original. G17 prints all 17 significant digits required to round-trip exactly back to the source f64. Apply the same fix to cal_match and lda_scores dumps. This eliminates what we previously thought was a real f32 cascade through XIC intensity reading on the snr column. The cal_match snr "5.24e-10 ULP floor" at values around 346025 was actually F17 truncating ~15 sig figs and the comparator parsing the truncated string back to a 9-ULP-off f64. After G17 the snr column is at 6.94e-18 cross-impl (essentially zero - sub-epsilon). Per-column status on Stellar Single cal_match after this fix: * apex_rt: 3.55e-15 (was bit-equal artifact, 137 rows at f64 noise) * correlation: bit-equal (was 4.97e-14) * libcosine: bit-equal (was 5.55e-16) * xcorr: 6.94e-18 (was 5.11e-15) * snr: 6.94e-18 (was 5.24e-10 - this was the F17 artifact) CAL_MATCH overall max_diff: 5.24e-10 -> 3.55e-15 (5 orders tighter). LDA_SCORES max_diff essentially unchanged at f64 epsilon. Boundary count holds at 5 PASS / 4 FAIL. All remaining FAILs now at single-ULP f64 noise.
Same pattern as DiannTsvLoader (11943b1) - .NET Framework double.TryParse can be off by 1-2 ULPs on 16-digit values whereas XmlConvert.ToDouble is required by XML schema spec to be IEEE-correct. Replaces all 13 double.TryParse callsites in MzmlReader with a TryParseXmlDouble helper. Effect on Stellar Single is marginal: .NET Framework 4.7.2's XmlConvert.ToDouble apparently has the same parser limitation as double.TryParse for the specific cvParam values that produce the apex_rt 137-row 2-ULP drift (e.g. "17.0286203070330" still parses to 1 ULP off from Rust's IEEE-correct parse). The fix is still correct in principle and will be relevant on .NET 5+ migration (where the underlying parser is IEEE-754 correct) and for any 16-digit cvParam values that fall in the parser's correctable range. Cross-impl boundary count unchanged at 5 PASS / 4 FAIL with the remaining apex_rt 2-ULP drift at the .NET Framework 4.7.2 parser precision floor for this dataset.
* TukeyMedianPolish.PearsonCorrelationRaw: switched to single-pass moment form to match Rust pearson_correlation_raw and existing PearsonCorrelation.Pearson helper. Brings median_polish_residual_correlation to cross-impl bit-equal. * ParquetScoreCache.WriteScoresParquet (both overloads): iterate entries in canonical (EntryId, Charge, ScanNumber) order so per-side parquets have identical physical row layout cross-impl. Isolates Stage 5+ from parquet-writer row-order differences. See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
…pass sidecar reload * ProteinFdr.RunFirstPassProteinFdr static helper extracted from FirstJoinTask so the rehydration path can run the same logic. * PerFileRescoreTask: in --join-at-pass=2 mode, recompute first-pass protein FDR before compaction (mirrors Rust pipeline.rs:4292 which is gated by !can_skip_fdr || expect_reconciled_input). The recompute is typically a no-op vs the loaded sidecar's RunProteinQvalue on single-file runs, but the call must exist so the compaction predicate sees the same RunProteinQvalue values the in-process pipeline computes. * MergeNodeTask: re-load .2nd-pass.fdr_scores.bin onto post-compaction stubs before RunProteinFdr. The detected_peptides gate filters on ExperimentPrecursorQvalue, which must be the 2nd-pass value to match Rust's reload-then-second-pass-FDR sequence. Without this, 19 borderline peptides leaked into the C# second-pass parsimony graph on Stellar Single, producing a 1-protein delta in Stage 7 picked-protein output. New OSPREY_DUMP_DETECTED_PEPTIDES env var (mirrors osprey Rust side) for future bisection. Brings per-side cross-impl Stage 7 + blib to PASS end-to-end on net8.0 Stellar Single with no information sharing between the two implementations. See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
Rust deduplicate_pairs ends with sort_by_key(|e| e.entry_id) (osprey pipeline.rs:6123) with an explicit comment: "Sort by entry_id for deterministic order regardless of HashMap iteration. Without this, the random HashMap order propagates to SVM feature matrix row ordering, causing non-deterministic gradient updates and model weights." The C# port was returning entries in Dictionary.Values insertion order instead. The rehydration path (--join-at-pass=2) masked this because entries loaded from the canonically-sorted parquet, but the straight-through in-memory path fed unsorted entries straight to Percolator, producing different SVM working-set selection cross-impl and ~190-precursor / ~270-peptide first-pass FDR drift on Stellar Single. With the matching sort, end-to-end straight-through cross-impl bit-parity at 1e-9 on Stellar Single: every Stage 7 protein FDR column 0 rows differ, every blib table column 0 rows differ. Drops fully-qualified System.* qualifiers in the MergeNodeTask diagnostic dump that the inspector flagged. See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
* ConsensusRts: pair decoys with targets by base_id (entry_id & 0x7FFFFFFF) instead of stripping a "DECOY_" prefix from modified_sequence. Mirrors Rust reconciliation.rs::compute_consensus_rts; per its explicit comment, prefix-strip is fragile (silently misses library-supplied decoys whose modified_sequence has no prefix; can leak decoys whose target instance happens to share a sequence). On Stellar 3-file, the prefix path leaked 16 extra decoys into the cross-file consensus output; with base_id pairing the consensus dump is now bit-equal cross-impl (Stage 6 PASS). * FirstJoinTask.RunPercolatorFdr: sort each file's entries by EntryId before building the Percolator input. The 1st-pass list is already entry_id-sorted via DeduplicatePairs (AbstractScoringTask), but post-rescore gap-fill entries can be appended after the sorted pre-existing rows. Re-sorting here mirrors Rust pipeline.rs::run_percolator_fdr and gives the SVM working-set selection a canonical iteration order regardless of gap-fill insertion order. 3-file Stellar per-side stages 1-4, 5, 6 now PASS cross-impl. Stage 7 still diverges (2nd-pass Percolator output drifts cross-impl on multi-file inputs in a way these two fixes don't fully close); investigation continues. See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
The comment previously read "Matches the Rust streaming Percolator path." Rust's direct path was patched to add the same dedup step (osprey-fdr/src/percolator.rs::run_percolator), so the C# direct path now mirrors BOTH Rust paths and the inline note documents why the dedup matters statistically (avoids the SVM seeing the same precursor N times in N-file experiments). No code change in this commit; the dedup itself was already present in the C# direct path. Co-Authored-By: Claude <noreply@anthropic.com>
* PercolatorEntry.Id now (file, mod_seq, charge, scan) like Rust * Per-file sort tie-breaks on ParquetIndex after EntryId/Charge/Scan * perc_input + subsample dumps gain native_position tie-break * Stage 7 cross-impl Stellar 3-file goes from FAIL to PASS See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
* Stellar Windows + WSL timing tables updated after 3-file straight-through bit-equality landed * Stellar Windows total: Rust 4:29, C# 4:39 (1.04x ~tied) * Stellar WSL total: Rust 6:01, C# 6:39 (1.11x Rust faster) * Astral columns unchanged (not re-measured this session) See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
* MzCalibration.CalculateSingleCalibration now uses Welford's online algorithm for mean and variance, matching the Rust port's switch to the same numerically stable formulation. The naive sum/n let the running accumulator drift to magnitude ~|n * mean| (~4000 on Astral file 49), where f64 ULP is ~5e-13 — enough that the Rust LLVM and .NET JIT, vectorising the loop differently, produced 1-ULP-different cal mean / sd cross-impl. Welford's loop-carried dependency on mean and m2 forces serial scalar execution in both runtimes. * Adds OSPREY_DUMP_MS2_CAL_ERRORS / OSPREY_MS2_CAL_ERRORS_ONLY env vars and a paired WriteMs2CalErrorsDump method that writes cs_ms2_cal_errors.txt for cross-impl diffing of the per-fragment calibration inputs. * PerFileScoringTask captures the filtered "contributing matches" list and fires the dump when the env var is set. Verified on Astral single file: all calibration.json scalars are now bit-equal cross-impl with the Rust port. See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
* ProteinFdr.ComputeProteinFdr now carries a SortKey (sorted accessions joined by ';') on each ProteinWinner and uses it as the cumulative-FDR sort tiebreak instead of GroupId. Matches the Rust port's change (osprey commit for the same fix). * The previous GroupId tiebreak was HashMap-iteration-order from BuildProteinParsimony, non-deterministic within C# across runs AND across the Rust ↔ OspreySharp boundary. Once upstream calibration became bit-equal, the tiebreak started actually firing on every score tie and produced group_qvalue divergence. * The winners.Sort call carries an inline "// Array.Sort OK:" exemption documenting why ties are unreachable (accessions strings are unique per group by parsimony construction). * See ai/docs/osprey-development-guide.md "Stable vs unstable sort" for the convention. See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
* OSPREY_DUMP_STAGE7_WINNERS emits cs_stage7_winners.tsv with rank, score, is_decoy, sort_key, raw_qvalue, monotonic_qvalue * OSPREY_DUMP_BEST_PEPTIDE_SCORES emits cs_best_peptide_scores.tsv with score, is_decoy, best_qvalue per peptide * Both env-var-gated, no-op in production. Mirror osprey-fdr/protein.rs additions Co-Authored-By: Claude <noreply@anthropic.com>
* Removed gf.DecoyEntryId from gapFillIds in PerFileRescoreTask * Removed decoy branch from forced-integration loop * Targets are still gap-filled (missing from this file by definition); decoys keep their 1st-pass natural-RT score * Mirrors osprey pipeline.rs fix; achieves Astral 3-file bit-parity at 1e-9 Co-Authored-By: Claude <noreply@anthropic.com>
* Created OspreySharp.FDR/FdrDiagnostics.cs with WriteStage7WinnersDump and WriteBestPeptideScoresDump (FDR project can't reference OspreyDiagnostics due to layering) * Moved WriteStage7DetectedPeptidesDump into OspreyDiagnostics with matching DumpDetectedPeptides static bool flag * ProteinFdr.cs and MergeNodeTask.cs callsites are now one-line gated calls; diagnostic file I/O stays out of production algorithm code * Caller-side flag check short-circuits the LINQ projection that builds the (Score, IsDecoy) tuple list before the disabled-dump path can pay any allocation cost Co-Authored-By: Claude <noreply@anthropic.com>
Mirrors the Rust change in osprey/crates/osprey-io/src/mzml/parser.rs (maccoss/osprey PR #39, commit 9905a2f). Both ToMs2Spectrum() and the linear convert path now throw an InvalidDataException naming the spectrum index and the missing cvParam (MS:1000828 or MS:1000829) instead of substituting the DEFAULT_ISOLATION_HALF_WIDTH = 12.5 hardcoded default. DIA processing cannot proceed without true isolation windows, and a silent default produces bogus results that are very hard to diagnose downstream. Removed the now-unused DEFAULT_ISOLATION_HALF_WIDTH constant. Co-Authored-By: Claude <noreply@anthropic.com>
* Updated Windows + WSL tables with 2026-05-24 single-run timings * Replaced stale SSD-vs-HDD speculation with measured I/O bench * Corrected ext4.vhdx location claim (now on D: HDD, not C: SSD) See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md (postscript 18) Co-Authored-By: Claude <noreply@anthropic.com>
* Saves ~21s on Stellar 3-file stage1to4 on the Rust mirror * ppm-scale mass errors have ~9 digits of headroom for naive sum * IEEE 754 strict mode prevents JIT vectorisation of dependent sum See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md (postscript 19) Co-Authored-By: Claude <noreply@anthropic.com>
* Three medians-of-3 tables: Windows C:, WSL /home (ext4), WSL /mnt/c (drvfs) * /home shows historical Rust 17-21% faster; /mnt/c shows C# at parity * Caveat block + I/O bench rewritten for current C:-SSD storage layout See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md (postscript 19) Co-Authored-By: Claude <noreply@anthropic.com>
* True medians via Math.Floor instead of PowerShell banker's-rounding [int] * C# stage7 now includes second-pass-fdr STAGE-WALL time (was dropped) * Stage7 ratios now reflect reality: C# 1.07-1.33x slower than Rust See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
* Move OSPREY_DUMP_LOESS_INPUT call from RunCalibrationScoringPass to caller * Pass 1 dump fires unconditionally; pass 2 dump only on R^2 acceptance * Matches Rust pipeline.rs: dump reflects the calibration actually used * Preserves OSPREY_LOESS_INPUT_ONLY early-exit after pass 1 See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
* PercolatorFdr.cs had a UTF-8 BOM introduced by an editor save that also corrupted the two em-dashes in comments at lines 769 and 1824 into mojibake (Windows-1252-misread UTF-8: U+00E2 U+20AC U+201D). Strip the BOM and replace both em-dashes with ASCII '--' to keep the file pure ASCII and immune to future encoding round-trips. * OspreySharp.sln also carried a UTF-8 BOM; strip for consistency with the rest of the OspreySharp folder. Verified via ai/scripts/validate-bom-compliance.ps1 -- the OspreySharp folder is now clean of unexpected BOMs (Skyline.sln.DotSettings BOM is outside the OspreySharp tree and pre-exists on master). C# build still passes. Co-Authored-By: Claude <noreply@anthropic.com>
…pped dead v1 hydration guard * MergeNodeTask: log warning when no parquet path mapped or row count < entry count * RescoreHydration: removed dead envelopeStems.Count>0 guard (v1 rejected at parse) See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <noreply@anthropic.com>
…geNodeTask keys * ReconciliationFile.Load: reject v2 envelopes with missing/empty file_stems * MergeNodeTask: warn when perFileEntries keys have no matching config.InputFiles * IOTest: updated 3 reconciliation fixtures to carry FileStems See ai/todos/active/TODO-20260516_ospreysharp_wsl_parity.md Co-Authored-By: Claude <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.
Companion to maccoss/osprey#37 (Rust HPC chain bit-parity fixes).
Closes the C# Stage 7 algorithmic divergence we noticed during
performance work and bundles in the prior parity-sprint perf
improvements that were waiting for these correctness fixes to land
together.
Why now
While measuring Astral pipeline performance we noticed an anomaly
that called the prior parity work into question. Two underlying
problems turned out to be hiding the issue:
MergeNodeTaskwas not running a 2nd-pass Percolator passat all -- the merge node persisted 2nd-pass FDR sidecars from
the in-memory entries' existing 1st-pass scores rather than
recomputing them on the post-reconciliation features. The
straight-through C# pipeline silently produced ~25% fewer
precursors than expected.
--join-at-pass=2had the same skip-2nd-pass bug, ANDworkers stamped a single-file reconciliation hash that the
merge node correctly rejected. Bit-parity between the in-process
Rust path and the HPC chain (sidecar files + rehydration) was
not actually being validated end-to-end.
Both made it impossible to get reliable performance numbers. We
returned to first principles: built a strict bit-parity test that
compares a 4-step HPC chain run (raw workers + first-join + per-file
rescore workers + 2nd-join merge) against a straight-through pipeline
run using real mzMLs + calibration, and fixed the bugs the test
surfaced on both sides.
What this PR contains
Correctness changes (Bug A/B/C/D port from Rust)
MergeNodeTasknow runs2nd-pass Percolator before persisting 2nd-pass sidecars when any
sidecar is missing (HPC distribution case). Required refactoring
FirstJoinTask.RunPercolatorFdrtointernal staticwithPipelineContext+passLabelparameters so MergeNodeTask canreuse the same Percolator harness, and reloading PIN features
from the reconciled parquets before the call (PerFileScoringTask's
bundle-hydration path explicitly nulls Features after stub load to
preserve
WriteReconciledParquet's "Features != null meansrescored" criterion).
ReconciliationFilegains aFileStemsfield;CurrentFormatVersionbumped to 2.
OspreyConfig.ReconciliationParameterHashForStemsadded (mirrors Rust).
FirstJoinTask.BuildReconciliationFilepopulates the field with the sorted+deduped multi-file stems set;
RescoreHydration.HydrateReconciliationOverlaycaptures andvalidates it across siblings (
JoinFileStemsonRescoreInputs).PerFileRescoreTask.WriteReconciledParquetstamps the join-widehash when stems are provided.
LoadMassCalibrationsnow throwsInvalidDataExceptiononmissing or unparseable
.calibration.json, instead of silentlydegrading to uncalibrated MS scoring.
Parity-sprint perf work (folded in)
Performance work from the weeks before the parity issue surfaced:
SVM grid-search parallelization (
LinearSvmClassifier,Matrix),XCorr scratch pool (
XcorrScratchPool,SpectralScorer), Percolatorhot-path improvements (
PercolatorFdr), pipeline instrumentation(
AnalysisPipeline).Cleanup
ReSharper inspection cleanup (was 24 warnings: redundant
qualifiers, redundant usings, ambiguous XML doc refs, one possible-
null assignment);
Array.Sortexemption documented on the onepermitted use in
PercolatorFdr.cs(tie-break key is unique perrow post-dedup, so introsort's instability is moot).
Updated
Osprey-workflow.htmlwith the corrected HPC boundarydiagrams reflecting the 4-step chain.
Validation
Cross-impl
Test-Regression.ps1(stage1to4 / stage5 / stage6 /stage7 / blib): all 5 gates PASS on Stellar single-file,
Stellar all-files, Astral single-file, and Astral all-files. The
Stage 5 percolator gate at Astral all-files is relaxed to per-column
1e-9 tolerance (was strict SHA-256) to absorb a 168-row ULP-1
drift in
experiment_precursor_qat that scale -- flagged forend-of-pipeline review in the ai/master companion change.
Cross-impl HPC-chain validation (companion ai/master scripts):
(60356/60373 precursors, hash matched, downstream Stage 6 drift
only); Astral PASS (165354/165288, same shape).
(60373/60373 exact, hash matched); Astral PASS (165288/165288
exact). Proves the C# WRITE side of the v2 envelope is
byte-equivalent to Rust's.
Pre-commit gates (
Build-OspreySharp.ps1 -RunInspection -RunTests):TestNoUnstableArraySortrule)Merge strategy
Single PR for review purposes; will be squashed at merge.