MSL mzLib Spectrum Library indexed binary format#1036
Merged
Conversation
…mentIon ReadFragmentIon_InternalIon_StartAndEndResidue_Correct only checked the start residue (FragmentNumber). Add the secondary product type and end residue (SecondaryProductType / SecondaryFragmentNumber) asserts the name already promised, so the text-parse path is proven to preserve the full internal-ion annotation — not just the binary round-trip test. Addresses PR smith-chem-wisc#1036 review comment (internal-ion annotation completeness). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MslProteoformScorer.MatchedFragments intentionally carries the library peak m/z/intensity, not the matched experimental peak — it is a library-vs-experiment scorer whose result identifies which library fragments matched. Experimental intensities feed only the spectral-angle score. Document this at the construction site to settle the ambiguity. Addresses PR smith-chem-wisc#1036 review comment (library vs experimental match values). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SpectralLibrary.cs had been reindented spaces -> tabs across the whole file, which inflated the PR diff by ~730 whitespace-only lines and buried the real .msl routing / internal-ion changes. Repo style is 4-space (no .editorconfig; master uses spaces). Convert leading tabs back to spaces; the file is byte-identical ignoring whitespace, so behavior is unchanged. PR diff for this file drops from 1010/758 to ~252/24. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TestAveragingSpectraWriteFile.cs (a spectra-averaging teardown rewrite) and TestDatabaseLoaders.cs (removal of an unused helper) are unrelated to the .msl spectral-library format and only widened this PR's surface. Revert both to master to keep the PR focused; the averaging-teardown improvement can land in its own PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nbollis
reviewed
Jun 10, 2026
Strip development-process residue that leaked into source: - Mslindex.cs: drop the "// New property/method to ADD alongside it" instruction comments and re-indent Count/GetEntryAt with tabs to match the file (they were pasted in space-indented); add real XML summaries. - MslLibrary.cs / MslReader.cs: reword four doc comments that referenced an internal "Prompt N / Prompt N Handoff" process, keeping the technical content. No behavior change. Addresses nbollis's smith-chem-wisc#1036 review (agent residue / mechanical assembly). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… residue
The MSL test suite encoded the agent's prompt-development sequence into 16
fixture names (TestMslPromptN...) and ~50 doc/category/string mentions of
"Prompt N". Rename every fixture to describe what it tests (e.g.
TestMslPrompt13GetAllLibrarySpectraFix -> TestMslGetAllLibrarySpectra;
collisions resolved as TestMslMergerCoverage / TestMslCoverageGapsExtended),
update all cross-references, drop the [Category("PromptN")] tags, de-prompt
temp-dir names, and reword the doc/assert text. Also fixes one stale source
reference in MslSpectralLibrary.cs to the renamed version-management test.
Test logic and assertions are unchanged.
Addresses nbollis's smith-chem-wisc#1036 review (agent residue / mechanical assembly).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
Re: the #1036 review (the one posted on #1038)Thanks @nbollis — went through every item. Dispositions below. Addressed
Already correct (no change needed)
Respectfully, not bugs — no change
Since the review landed on #1038, flagging here so the threads track against the right PR. Happy to re-request review on #1036 once CI is green. |
The PR had bumped the nuspec <version> to a dev/local-feed value (9.9.901) used while building local packages for the MetaMorpheus consumer PRs. Revert to master's 1.0.574 so the package version isn't inflated on merge; release versioning is handled separately. Addresses nbollis smith-chem-wisc#1036 review comment ("Change Nuspec back"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s override The override ran batched Torch forward passes for speed, but its results are not bit-identical to per-peptide prediction (~1e-7 drift from batched BatchNorm/matmul), which broke two pre-existing exact-equality tests (PredictRetentionTimeEquivalents_MatchesSinglePredictions and _LargeBatch) and contradicted its own "results identical to PredictCore" doc. It is not needed here: the method is declared on IRetentionTimePredictor and implemented in the base RetentionTimePredictor, so Chronologer now inherits the base implementation. The consumers (MetaMorpheus parallel search RT calibration) call the interface method and keep working; only a one-time calibration step loses the batched speedup. This file now matches master. Batched inference can return as its own PR with tolerance tests. Addresses nbollis smith-chem-wisc#1036 review ("Is this method needed? ... use the existing method"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… corruption Building spectral libraries calls Chronologer for millions of peptides. The ~30 tensor intermediates allocated inside each forward() were relying on the GC finalizer thread for disposal, which races libtorch's native allocator and corrupts the heap (0xC0000374) once enough inference has run. Wrap each Predict chunk (and the single-peptide path) in NewDisposeScope() so those tensors are freed deterministically on the calling thread. Also adds an opt-in CUDA path (MZLIB_CHRONOLOGER_CUDA=1; default CPU, numerically equivalent) behind a Device property; inert unless the env var is set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The batched PredictRetentionTimeEquivalents override runs the model at batch size m, while PredictRetentionTimeEquivalent runs it at batch size 1. libtorch selects different conv/matmul kernels for the two paths, so predictions agree only to float32 precision (~1e-7 relative), not bit-exactly. Two assertions used Is.EqualTo with no tolerance and failed on that drift. Add .Within(1e-4) - orders of magnitude above the noise floor yet tight enough to still catch a genuine batch/single mismatch. Also soften the override docstring, which claimed the two paths give identical results. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 15, 2026
trishorts
added a commit
to trishorts/mzLib
that referenced
this pull request
Jun 15, 2026
The base RetentionTimePredictor.PredictRetentionTimeEquivalents parallelizes single-peptide PredictCore calls across CPU threads — each peptide is still a separate, lock-held, batch-of-1 libtorch forward pass. Override it for Chronologer with true tensor batching: format/encode the peptides in parallel on the CPU, then run the model in large batched forward passes (one [m,1] tensor per chunk, model lock taken once per chunk instead of once per peptide). The model is in eval mode (BatchNorm running stats), so each peptide's prediction is independent of the batch; results match PredictCore to float32 precision (libtorch picks different conv/matmul kernels at batch size m vs 1). Also wraps the forward pass in NewDisposeScope() so its native intermediates are freed deterministically. Test tolerance widened to allow the float32 drift. Extracted from smith-chem-wisc#1036 so the batched-inference work lands independently of the .msl format changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ictor Remove the MZLIB_CHRONOLOGER_CUDA opt-in and the Device plumbing: the model always runs on CPU and the batched override no longer moves tensors to a device. An env-gated execution mode is the wrong surface, the GPU path was never used in the production library build, and if GPU inference is wanted later it should be an explicit constructor parameter with benchmarks. Keeps the batched-CPU override and the NewDisposeScope() native-heap fix. Mirrors smith-chem-wisc#1077. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
trishorts
added a commit
to trishorts/mzLib
that referenced
this pull request
Jun 15, 2026
…Search Offline library compiler for MetaMorpheus's ManySearchTask: digests protein FASTAs once and writes compact indexed .msl libraries (precursor mass + charge + Chronologer iRT + sequence, lean / fragment-less by default) so the parallel search reads pre-built libraries instead of re-digesting thousands of FASTAs at search time. Supports per-database output, a merged db|accession-tagged index, and size-balanced sharded production builds with a global persisted iRT cache. Added as a new project under mzLib/MslDbBuilder referencing UsefulProteomics- Databases and Readers. Depends on the .msl writer/reader API from smith-chem-wisc#1036, so its base is mzlib_speclib. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…guard Document why MslFragmentIon is a standalone persistence/working model rather than a subclass of or interface over the MatchedFragmentIon domain type (different layer, mutable float32 storage record, no eager Product / neutral mass, carries storage-only ExcludeFromQuant; the two are bridged only by explicit conversion). Also document why the two annotation renderings intentionally differ. Add MslFragmentIonInteropTests: round-trips a diverse fragment set (terminal, neutral-loss, internal, diagnostic) through ToLibrarySpectrum/FromLibrarySpectrum asserting every mapped field survives, so the two models can't silently drift; and pins that ExcludeFromQuant is dropped by the domain projection (it round-trips only through the binary .msl format). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
trishorts
added a commit
to trishorts/mzLib
that referenced
this pull request
Jun 15, 2026
…Search Offline library compiler for MetaMorpheus's ManySearchTask: digests protein FASTAs once and writes compact indexed .msl libraries (precursor mass + charge + Chronologer iRT + sequence, lean / fragment-less by default) so the parallel search reads pre-built libraries instead of re-digesting thousands of FASTAs at search time. Supports per-database output, a merged db|accession-tagged index, and size-balanced sharded production builds with a global persisted iRT cache. Added as a new project under mzLib/MslDbBuilder referencing UsefulProteomics- Databases and Readers. Depends on the .msl writer/reader API from smith-chem-wisc#1036, so its base is mzlib_speclib. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
trishorts
added a commit
that referenced
this pull request
Jun 16, 2026
…1077) The base RetentionTimePredictor.PredictRetentionTimeEquivalents parallelizes single-peptide PredictCore calls across CPU threads — each peptide is still a separate, lock-held, batch-of-1 libtorch forward pass. Override it for Chronologer with true tensor batching: format/encode the peptides in parallel on the CPU, then run the model in large batched forward passes (one [m,1] tensor per chunk, model lock taken once per chunk instead of once per peptide). The model is in eval mode (BatchNorm running stats), so each peptide's prediction is independent of the batch; results match PredictCore to float32 precision (libtorch picks different conv/matmul kernels at batch size m vs 1). Also wraps the forward pass in NewDisposeScope() so its native intermediates are freed deterministically. Test tolerance widened to allow the float32 drift. Extracted from #1036 so the batched-inference work lands independently of the .msl format changes. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nbollis
previously approved these changes
Jun 16, 2026
Drop the ExternalReaders/Python and ExternalReaders/Rust reference readers (msl_reader.py, test_msl_reader.py, msl_reader.rs); they were interop reference code, not part of the C# build. Scrub the now-dangling version-sync references: trim the new-version checklist in MslSpectralLibrary.cs and repurpose the version canary test as FormatVersion_MustMatchExpectedConstant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Removed the Python and Rust external MSL reader implementations ( |
pcruzparri
approved these changes
Jun 18, 2026
nbollis
approved these changes
Jun 18, 2026
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.
Add
.msl— mzLib Binary Spectral Library FormatThis PR introduces
.msl(mzLib Spectral Library), a new binary spectral library format designed as a high-performance alternative to the existing text-based.mspformat for use in mzLib and MetaMorpheus workflows.Motivation
The
.mspformat requires full sequential parsing on every load, allocates heavily due to string-based fragment annotations, and cannot support random-access fragment retrieval for large libraries. The.mslformat addresses these limitations with a compact, structured binary layout that enables zero-copy struct reads, O(1) DDA lookups, zero-allocation m/z range queries, and an index-only load mode for multi-gigabyte libraries.What's Included
New binary format (
Omics+Readersprojects)MslFormat— all format constants, enums, and bitfield encode/decode helpersMslStructs— five fixed-layout binary structs (Pack = 1) covering header, protein record, precursor record, fragment record, and footerMslFragmentIon/MslLibraryEntry— rich in-memory models with full ion annotation (internal fragments, neutral losses, diagnostic ions, oligonucleotides, proteoforms)MslWriter— two-pass atomic writer (temp file + rename); intensity normalization per precursor; fragment m/z sortMslReader— binary reader with CRC-32 validation and both full-load and index-only modesMslIndex— in-memory query engine: m/z-sorted array for range queries, O(1) sequence/charge dictionary, elution group map, LRU fragment cacheMslLibrary— top-level user façade withLoad,LoadIndexOnly,Save,SaveFromLibrarySpectra,QueryMzWindow,QueryWindow(m/z + RT + IM filters),TryGetEntry,WithCalibratedRetentionTimesMslFileTypeHandler— registers the.mslextension with mzLib's file-type routingSpectralLibrary.cs— updated to route.mslfiles throughMslLibrarytransparentlyFormat highlights
MZLB+ CRC-32 integrity check + footer trailing magic for truncation detectionMoleculeTypeenum)Tests
100+ NUnit 4 tests across 8 test classes covering writer round-trips, reader validation (including CRC corruption and truncation detection), index queries, regression against a known-good MSP reference file (42 precursors / 968 fragments), and edge cases (empty libraries, Unicode sequences, large fragment counts).
Benchmarks (BenchmarkDotNet, Intel Core i7-8550U, .NET 8, 1 K–500 K precursors at 10 fragments each)