Skip to content

Implement Binary Spectrum Library Form .msl#2630

Open
trishorts wants to merge 11 commits into
smith-chem-wisc:masterfrom
trishorts:mslLibrary
Open

Implement Binary Spectrum Library Form .msl#2630
trishorts wants to merge 11 commits into
smith-chem-wisc:masterfrom
trishorts:mslLibrary

Conversation

@trishorts

Copy link
Copy Markdown
Contributor

binary format (.msl) is created, updated and used in search. Binary libraries also visualized in MetaDraw butterfly plots. Presently both msp and msl are co-written.

Alexander-Sol and others added 11 commits February 28, 2023 11:29
# Conflicts:
#	MetaMorpheus/CMD/CMD.csproj
#	MetaMorpheus/EngineLayer/EngineLayer.csproj
#	MetaMorpheus/GUI/GUI.csproj
#	MetaMorpheus/GuiFunctions/GuiFunctions.csproj
#	MetaMorpheus/TaskLayer/MetaMorpheusTask.cs
#	MetaMorpheus/TaskLayer/TaskLayer.csproj
#	MetaMorpheus/Test/Test.csproj
- perf: WriteSpectrumLibrary converted every spectrum to an MSL entry
  twice (the first materialization was dead code, overwritten before
  use); convert once and derive the saved entries from that result.
- UpdateSpectralLibrary now warns when it skips invalid MSL entries,
  matching WriteSpectrumLibrary; previously entries could silently
  vanish from the .msl consumed by search while .msp kept them.
- Remove duplicate using directives (EngineLayer.DIA/SpectrumMatch/Util
  appeared 2-3x after the merge) and revert tab-vs-space indentation
  churn in DbForTask and ReportProgress.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gcrsvLBgMKmDNpnbM3iE8
- TestWriteSpectrumLibrary: the co-write now emits both a .msp and a
  .msl; the old assertion grabbed the first SpectralLibrary file
  (which sorts to the .msl) and failed .Contains(.msp). Assert both
  formats are present instead.
- Add MslSpectralLibraryWriteTests exercising WriteSpectrumLibrary and
  UpdateSpectralLibrary: both formats are co-written and entries that
  fail conversion are skipped from the .msl (the warn branch) while the
  two valid entries round-trip through SpectralLibrary.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gcrsvLBgMKmDNpnbM3iE8
@trishorts

Copy link
Copy Markdown
Contributor Author

Quality summary

This PR adds the open binary spectral-library format .msl to MetaMorpheus as a peer of the
existing text .msp, wired through the same spectral-library machinery. Below is a summary of the
change and the verification behind it.

What it does

  • RecognitionDbForTask classifies .msl (alongside .msp) as a SpectralLibrary database entry.
  • Consumption.msl can be used as a spectral library in a search, loaded in the main-window
    database/library UI, and rendered in MetaDraw mirror plots, on par with .msp.
  • ProductionWriteSpectrumLibrary / UpdateSpectralLibrary now co-write both formats
    (SpectralLibrary_*.msp and SpectralLibrary_*.msl). Co-write is an intentional transitional
    state; the binary format's on-disk serialization lives in mzLib, not this PR.

Verification

  • Up to date with master — merged current upstream/master; conflicts resolved (dependency +
    using directives). MERGEABLE, no conflicts.
  • mzLib pinned to 1.0.582 — the published release that provides the .msl reader/writer
    (MslLibrary, MslLibraryEntry), applied consistently across all projects.
  • Builds clean — full solution builds in Release with 0 errors.
  • Code review addressed — a structured review of the four changed source files produced six
    findings, all resolved:
    • (perf) WriteSpectrumLibrary converted every spectrum to an MSL entry twice (the first
      materialization was dead code overwritten before use) → convert once.
    • (correctness) UpdateSpectralLibrary silently dropped entries that failed conversion → it now
      warns with the skipped count, matching WriteSpectrumLibrary, so the .msl/.msp pair
      can't diverge unnoticed.
    • (hygiene) removed duplicate using directives and reverted incidental tab/space churn.

Test coverage

  • Patch coverage: 100% of changed executable lines (33/33), measured with Coverlet in Release —
    the same instrumenter Codecov consumes — comfortably clearing the 90% gate.
  • Existing integration tests already exercise the happy-path co-write (WriteSpectralLibrary=true /
    UpdateSpectralLibrary=true across search, glyco, XL and MetaDraw tests).
  • Fixed TestWriteSpectrumLibrary, which asserted the first SpectralLibrary* file was the .msp;
    with co-write the .msl now sorts first, so it asserts both formats are emitted.
  • Added MslSpectralLibraryWriteTests covering the previously-untested invalid-entry path: an
    entry that fails conversion is skipped from the .msl (with a warning) while the .msp retains
    every spectrum; the two valid entries round-trip back through SpectralLibrary.

Notes for review

  • The .msl binary encoding/versioning is out of scope here (it lives in mzLib 1.0.582).
  • Co-write (.msp + .msl) is deliberate and transitional, not redundancy to remove in this PR.

Sync, review, fixes, and the added tests were prepared with Claude Code assistance (see the
Co-Authored-By trailers); all changes were built and tested locally before pushing.

@trishorts

Copy link
Copy Markdown
Contributor Author

Manual unit-test review

All 2 unit test(s) added by this PR were reviewed individually by the author (not only by automated/AI-generated checks): 2 approved as written, 0 edited, 0 flagged.

This satisfies the author's standing AI-use policy that AI-assisted tests receive human review before relying on them. Recorded 2026-07-11 10:37.

trishorts added a commit to trishorts/MetaMorpheus that referenced this pull request Jul 11, 2026
Align with the published mzLib release used by the .msl work in smith-chem-wisc#2630.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gcrsvLBgMKmDNpnbM3iE8
@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.30%. Comparing base (3c8e0b8) to head (821eb7f).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2630   +/-   ##
=======================================
  Coverage   93.29%   93.30%           
=======================================
  Files         196      196           
  Lines       21320    21340   +20     
  Branches     3994     3997    +3     
=======================================
+ Hits        19891    19911   +20     
  Misses        893      893           
  Partials      536      536           
Flag Coverage Δ
unittests 93.30% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...aMorpheus/EngineLayer/DatabaseLoading/DbForTask.cs 100.00% <100.00%> (ø)
MetaMorpheus/TaskLayer/MetaMorpheusTask.cs 86.68% <100.00%> (+0.29%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants