Skip to content

Real OpenMS FLASHDeconv using exe from provided installation path#1045

Open
trishorts wants to merge 20 commits into
smith-chem-wisc:masterfrom
trishorts:realFlashDecon
Open

Real OpenMS FLASHDeconv using exe from provided installation path#1045
trishorts wants to merge 20 commits into
smith-chem-wisc:masterfrom
trishorts:realFlashDecon

Conversation

@trishorts

Copy link
Copy Markdown
Contributor

Add RealFLASHDeconvolution — wrapper for the official FLASHDeconv executable (OpenMS)

Summary

This PR adds RealFLASHDeconvolution as a new first-class deconvolution method in mzLib. It wraps the official FLASHDeconv executable from OpenMS behind the standard DeconvolutionParameters / DeconvolutionAlgorithm / Deconvoluter interface, so MetaMorpheus can use it transparently alongside Classic, IsoDec, and the reverse-engineered FLASHDeconvolution.

The design follows the IsoDecAlgorithm pattern — an external binary invoked at runtime, with results converted to IsotopicEnvelope objects. IsoDec uses a DLL via P/Invoke; RealFLASHDeconvolution uses FLASHDeconv.exe via System.Diagnostics.Process.


Motivation

mzLib already contains a reverse-engineered pure-C# implementation of FLASHDeconv (FLASHDeconvolutionAlgorithm). This PR gives users access to the official validated binary from OpenMS without leaving the mzLib/MetaMorpheus pipeline, enabling direct comparison and providing a gold-standard reference implementation.


Files Added

File Location Description
RealFLASHDeconvolutionParameters.cs MassSpectrometry/Deconvolution/Parameters/ User-configurable parameters; maps to FLASHDeconv CLI flags
RealFLASHDeconvolutionAlgorithm.cs MassSpectrometry/Deconvolution/Algorithms/ Writes temp mzML, runs FLASHDeconv.exe, parses TSV output
TestRealFLASHDeconvolutionInfrastructure.cs Test/Deconvolution/ Infrastructure + real-exe tests (11 tests, all passing)

Files Modified

File Change
DeconvolutionType.cs Added RealFLASHDeconvolution enum value
Deconvoluter.cs Added RealFLASHDeconvolution case to factory switch

Architecture

DeconvolutionType.RealFLASHDeconvolution
        │
        ▼
Deconvoluter.CreateAlgorithm()
        │
        ▼
RealFLASHDeconvolutionAlgorithm
        │
        ├─ Per-scan:   Deconvolute(MzSpectrum, MzRange)
        │              → writes temp mzML → runs FLASHDeconv.exe → parses _ms1.tsv
        │
        └─ Whole-file: DeconvoluteFile(mzmlPath, parameters)   [preferred]
                       → passes full mzML directly → parses _ms1.tsv
                       → returns Dictionary<int scanNum, List<IsotopicEnvelope>>

DeconvoluteFile is the preferred mode. FLASHDeconv's multi-scan feature tracing only works on complete files. The per-scan Deconvolute() path exists for compatibility with the standard Deconvoluter interface but will produce sparse results on simple spectra.


Usage

Minimal

var p = new RealFLASHDeconvolutionParameters(
    flashDeconvExePath: @"C:\Program Files\OpenMS-3.0.0\bin\FLASHDeconv.exe");

// Whole-file (preferred)
var byScan = RealFLASHDeconvolutionAlgorithm.DeconvoluteFile(mzmlPath, p);

// Per-scan via standard interface
var envelopes = Deconvoluter.Deconvolute(scan, p);

Exe auto-detection

The algorithm searches these locations automatically if FLASHDeconvExePath is null:

  • C:\Program Files\OpenMS-3.x.x\bin\FLASHDeconv.exe (several versions)
  • /usr/bin/FLASHDeconv, /usr/local/bin/FLASHDeconv
  • System PATH

FLASHDeconv CLI flags (verified against OpenMS 3.0.0-pre-HEAD-2023-06-17)

Parameter Flag
Input mzML -in
Feature TSV (required) -out
Per-spectrum TSV (MS1, MS2) -out_spec ms1.tsv ms2.tsv
Tolerance -Algorithm:tol
Charge range -Algorithm:min_charge / -Algorithm:max_charge
Mass range -Algorithm:min_mass / -Algorithm:max_mass
Cosine threshold -Algorithm:min_isotope_cosine

Implementation Notes

mzML writer

The algorithm contains a self-contained mzML writer. Several requirements were found empirically against the OpenMS 3.0 pre-release parser:

  • No UTF-8 BOMnew UTF8Encoding(encoderShouldEmitUTF8Identifier: false). OpenMS rejects BOM silently by crashing with a misleading error.
  • <run> element must have defaultInstrumentConfigurationRef="IC1" — in addition to the <spectrum> element.
  • softwareList, instrumentConfigurationList (id=IC1), and dataProcessingList (id=dp) are all required.

-out_spec requires two paths for mixed MS1/MS2 files

Passing a single path to -out_spec when the input contains both MS1 and MS2 causes FLASHDeconv to write MS2 output to that path and produce no MS1 output. Two paths must always be passed; we read only the MS1 one.

Trailing tab in TSV header

FLASHDeconv writes a trailing \t after the last column name in the header row. The TSV parser strips empty strings from the split header with .Where(h => !string.IsNullOrEmpty(h)) to prevent every data row from being rejected by a length mismatch.

Single-peak envelopes

The standard -out_spec TSV provides per-envelope summary rows, not individual isotope peak m/z values. Each returned IsotopicEnvelope therefore contains a single peak at the representative m/z (midpoint of RepresentativeMzStart and RepresentativeMzEnd). For full isotope peak lists, use -out_topFD with the existing MsAlign reader.

Score mapping

Qscore is used as the envelope score when present; MassSNR is used as a fallback; IsotopeCosine is the final fallback.


Tests

All 11 tests pass. Tests are in TestRealFLASHDeconvolutionInfrastructure.cs.

Infrastructure tests (no exe required):

  • Parameters class defaults and explicit values
  • DeconvolutionType.RealFLASHDeconvolution enum value exists
  • Empty spectrum short-circuits without calling exe
  • Bad exe path throws FileNotFoundException

Real-exe tests (require FLASHDeconv at known path or FLASHDECONV_EXE env var):

  • RealFLASH_RealExe_WholeFile_SmallYeast_ReturnsEnvelopesDeconvoluteFile on SmallCalibratibleYeast.mzml returns envelopes
  • RealFLASH_RealExe_WholeFile_SmallYeast_EnvelopeCountMatchesReference — count matches reference run (3149 ± 5)
  • RealFLASH_RealExe_WholeFile_SmallYeast_EnvelopeFieldsValid — all field validity checks
  • RealFLASH_RealExe_WholeFile_SmallYeast_MassesInRange — all masses within requested bounds

Required User Setup

  1. Download and install OpenMS from openms.de
  2. Set RealFLASHDeconvolutionParameters.FLASHDeconvExePath to the FLASHDeconv binary, or add the OpenMS bin/ directory to PATH

For MetaMorpheus integration, GlobalSettings.FLASHDeconvExecutablePath should be added and wired to a Browse button in the Settings UI (deferred to a follow-up PR).


Known Limitations

  • Speed: Each Deconvolute() call launches a new process and writes/reads temp files. For whole-file work, use DeconvoluteFile() directly.
  • Single-peak envelopes: Use RetainMsAlignOutput = true + the MsAlign reader for full isotope peak lists.
  • MS2 not wired: Only the _ms1.tsv file is parsed. MS2 support is a straightforward extension.
  • Scan number extraction: SmallCalibratibleYeast.mzml (and similar files) produce ScanNum = 0 for all rows because FLASHDeconv cannot parse the native scan ID format. Data is still returned correctly, just all keyed to scan 0.

References

  • Jeong et al. (2020) Cell Systems 10(2):213–218.e6. DOI: 10.1016/j.cels.2020.01.003
  • OpenMS source: src/openms/source/ANALYSIS/TOPDOWN/SpectralDeconvolution.cpp

@codecov

codecov Bot commented Apr 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.83333% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.85%. Comparing base (78275fb) to head (16b1c96).

⚠️ Current head 16b1c96 differs from pull request most recent head 41c1c60

Please upload reports for the commit 41c1c60 to get more accurate results.

Files with missing lines Patch % Lines
...tion/Algorithms/RealFLASHDeconvolutionAlgorithm.cs 95.31% 0 Missing and 9 partials ⚠️
...nvolution/Algorithms/FlashDeconvExePathRegistry.cs 96.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1045      +/-   ##
==========================================
- Coverage   82.33%   81.85%   -0.49%     
==========================================
  Files         382      366      -16     
  Lines       50509    47050    -3459     
  Branches     6102     5584     -518     
==========================================
- Hits        41585    38511    -3074     
+ Misses       7734     7449     -285     
+ Partials     1190     1090     -100     
Files with missing lines Coverage Δ
...Lib/MassSpectrometry/Deconvolution/Deconvoluter.cs 100.00% <100.00%> (ø)
...ion/Parameters/RealFLASHDeconvolutionParameters.cs 100.00% <100.00%> (ø)
...nvolution/Algorithms/FlashDeconvExePathRegistry.cs 96.00% <96.00%> (ø)
...tion/Algorithms/RealFLASHDeconvolutionAlgorithm.cs 95.31% <95.31%> (ø)

... and 46 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…LASHDeconvolution

  - RunFLASHDeconv: drain stdout/stderr asynchronously before WaitForExit so
    the timeout actually applies and a chatty stderr can't deadlock the child
  - Quote -out_spec paths so spaces in WorkingDirectory don't truncate args
  - Use correct PSI-MS accession MS:1000523 for "64-bit float" cvParams in
    the synthetic mzML writer (was MS:1000515, the intensity-array accession)
  - Drive score-priority cascade by parse-success rather than non-zero value;
    Qscore=0 now means "use Qscore=0", not "fall through to SNR"
  - Document per-scan range-filter midpoint behavior; point users to
    DeconvoluteFile for full-fidelity output
  - Remove leftover [FD] Console.WriteLine debug logs and dead commented-out
    duplicate return in DeconvoluteFile
  - Tests: widen reference-count assertion to a sanity floor so OpenMS
    version drift doesn't trip CI; add exe-free unit tests covering the
    trailing-tab TSV header workaround, the no-BOM/instrumentConfigRef mzML
    invariants, negative-polarity sign-flip in the parser, and the
    Qscore→SNR→IsotopeCosine score-priority contract

  Bumped ParseSpecTsvByScan and WriteSingleScanMzml from private to internal
  to enable direct unit testing (Test has InternalsVisibleTo on MassSpectrometry).
  DeconvolutionParameters declares an abstract ToDecoyParameters() that all
  concrete subclasses must implement. The new RealFLASHDeconvolutionParameters
  on this branch hadn't grown the override yet, breaking the build.

  Returns null because decoy deconvolution (used for randomization-based
  FDR estimation) doesn't apply to the FLASHDeconv exe wrapper -- the
  class shells out to an external OpenMS executable. Mirrors
  ExampleNewDeconvolutionParametersTemplate, which made the same call.
  Add 3 NUnit tests exercising the deterministic failure modes that don't
  require the FLASHDeconv exe:

  - DeconvoluteFile_MissingMzml_ThrowsFileNotFound -- pins the input-file
    guard at the top of DeconvoluteFile so a typo'd mzML path produces a
    clear error instead of an opaque exe parse failure.
  - ParseSpecTsvByScan_RequiredColumnMissing_ThrowsMzLibException -- pins
    the required-column contract; missing ScanNum (or any other required
    column) must throw with the tried-name list, not silently produce zero
    envelopes.
  - ParseSpecTsvByScan_OptionalColumnsAbsent_ParsesAndUsesFallbacks -- pins
    the graceful-degradation behavior when RepresentativeMzEnd, Qscore, and
    MassSNR are all absent from the header (Score falls through to
    IsotopeCosine).

  Also document the accepted coverage gaps in the fixture's class-level
  doc: ResolveExePath null-path fallback (env-dependent / flaky on dev
  machines with OpenMS installed) and Deconvolute / RunFLASHDeconv
  process-invocation paths (covered by RealFLASH_RealExe_* integration
  tests when FLASHDECONV_EXE is set).
  Add one test that exercises the InvalidOperationException at
  Deconvoluter.DeconvoluteWithDecoys lines 64-67: when the supplied
  parameter class returns null from ToDecoyParameters() (i.e. doesn't
  support decoy deconvolution), the consumer must throw with a message
  naming the offending parameter type so the caller knows what to
  override. Uses RealFLASHDeconvolutionParameters (which intentionally
  returns null since the OpenMS-exe wrapper has no decoy concept) and an
  empty spectrum so no exe is required to reach the guard.
  The class previously locked all of its orchestration behind direct calls
  to a hardcoded exe-path resolver and an inline Process.Start, so codecov
  reported 56% on this file -- below the 90% bar. Refactor introduces two
  seams that production behavior keeps using by default but tests can swap:

  - FLASHDeconvRunner delegate: a constructor parameter (and matching
    DeconvoluteFile static overload) that defaults to the real-Process
    implementation. Tests inject a stub that writes canned TSV files to the
    requested output paths so Deconvolute / DeconvoluteFile orchestration
    (mzML write, range filter, parse, missing-MS1 handling) is exercised
    without launching a subprocess.
  - ResolveExePath testable overload: accepts the well-known-paths list and
    PATH-env value as parameters. The single-arg form delegates to it with
    the compiled-in DefaultWellKnownPaths and Environment.PATH so prod
    behavior is unchanged. Tests pass empty/known values to deterministically
    hit the well-known-search, PATH-search, and not-found-anywhere branches.

  The renamed RunFLASHDeconvDefault is marked [ExcludeFromCodeCoverage]
  because every line is pure subprocess plumbing -- testing it requires
  either the real FLASHDeconv exe (covered by the existing
  RealFLASH_RealExe_* integration tests when OpenMS is installed) or a
  Process abstraction that's out of scope.

  Add 9 NUnit tests covering the unlocked paths:
    - 3 ResolveExePath tests (well-known found, PATH found, not found)
    - 5 stub-runner orchestration tests on Deconvolute / DeconvoluteFile
    - 1 wrong-parameter-type guard test

  Lifts RealFLASHDeconvolutionAlgorithm.cs from 56% to 100% line coverage.
  Production callers (Deconvoluter.CreateAlgorithm) are unchanged -- the
  new constructor parameter is optional with the original constructor
  preserved for source compat.
Alexander-Sol
Alexander-Sol previously approved these changes May 1, 2026
  Group A of the PR smith-chem-wisc#1045 review pass.

  - TestRealFLASHDeconvolutionInfrastructure: tighten the
    EnvelopeCountMatchesReference assertion from "> 1000" to
    "Is.EqualTo(3149).Within(150)" so a regression cutting the output by
    half no longer silently passes -- matching the test name's promise.
    Comment documents the calibration to OpenMS-3.0.0-pre-HEAD-2023-06-17
    so contributors on a different version know to either update the
    constant or run with FLASHDECONV_EXE pointed at the reference build.

  - Replace Assume.That(allEnvs.Count, Is.GreaterThan(0)) with Assert.That
    in EnvelopeFieldsValid and MassesInRange so a zero-envelope regression
    is a hard failure, not a silent Inconclusive that gets waved off in CI
    noise.

  - Add ParseSpecTsvByScan_HeaderOnly_ReturnsEmptyDictionary covering the
    lines.Length < 2 early-return.

  - Add ParseSpecTsvByScan_RowsWithUnparseableRequiredCells_AreSilentlySkipped
    pinning the per-row "skip cell-parse failures, return what's valid"
    contract for real FLASHDeconv output that contains "NA" cells.

  - Add Deconvolute_StubRunner_RangeFilterIsInclusiveOnUpperBound pinning
    the boundary semantic of MzRange.Contains (which DoubleRange documents
    as inclusive) at the upper edge.

  - Add Deconvolute_EmptySpectrum_RunnerIsNotInvoked using the new
    FLASHDeconvRunner injection to assert directly that the runner is
    never invoked on an empty spectrum, instead of relying on the indirect
    side effect of an unresolvable exe path (the existing companion test).
RayMSMS
RayMSMS previously approved these changes May 1, 2026
  Each Deconvoluter.Deconvolute(scan, params) call constructs a fresh
  RealFLASHDeconvolutionAlgorithm and runs ResolveExePath, which hits
  File.Exists on the explicit path -- or walks the well-known list and
  PATH on misses. On a per-scan loop that's redundant filesystem syscalls
  per minute; the exe doesn't move between calls.

  - New public FlashDeconvExePathRegistry static class (ConcurrentDictionary
    of validated paths) with a Register(path) entry point. Production
    callers (e.g. MetaMorpheus from GlobalSettings.FLASHDeconvExecutablePath)
    can prime the registry once at startup so every Deconvolute call
    becomes lookup-only. Ad-hoc callers don't have to do anything --
    ResolveExePath caches lazily on first miss either way.

  - ResolveExePath now consults the registry first; on miss, runs the
    existing validate-and-resolve and caches the result. The 3-arg
    internal overload used by tests for deterministic injection is
    unchanged -- rubric tests stay isolated from cache state.

  - Cache key is the explicit path string, or a sentinel for the null
    default-search case. ConcurrentDictionary for thread-safety across
    parallel deconvolutions.

  - Cache invalidation is 'never' by design: the cached 'validated' state
    is an optimization, not a correctness contract. If the exe gets
    deleted at runtime, Process.Start surfaces the failure with the same
    error, just deferred. Internal Clear() exists only for test isolation.

  - 4 new tests in section E0 of TestRealFLASHDeconvolutionInfrastructure:
    Register valid / missing / null-or-empty inputs, plus an end-to-end
    test that registers a path, deletes the file, and asserts Deconvolute
    still proceeds (proves the cache is consulted, not the live filesystem).
@trishorts

trishorts commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

Please compare the diff to ensure that you are meeting all of the contractual obligations.

mzLib Spectral Deconvolution — Contractual Obligations

Distilled from the current wiki page:
https://github.com/smith-chem-wisc/mzLib/wiki/Spectral-Deconvolution-(Decharging-and-Deisotoping)

Last edited: Nov 25, 2025 (2 revisions)


1. Supported Algorithms

The wiki formally documents two supported algorithms:

Algorithm Class Parameters Class
Classic Deconvolution ClassicDeconvolutionAlgorithm ClassicDeconvolutionParameters
IsoDec IsoDecAlgorithm IsoDecDeconvolutionParameters

Obligation: Any new algorithm must be listed in this table with a description of what it is best for and its key features.


2. Architecture Contracts

2.1 Deconvoluter (Static Context Class)

Required public API:

public static IEnumerable<IsotopicEnvelope> Deconvolute(
    MzSpectrum spectrum,
    DeconvolutionParameters deconvolutionParameters,
    MzRange rangeToGetPeaksFrom = null);

public static IEnumerable<IsotopicEnvelope> Deconvolute(
    MsDataScan scan,
    DeconvolutionParameters deconvolutionParameters,
    MzRange rangeToGetPeaksFrom = null);

Obligation: Both overloads must exist and be public. The factory pattern (CreateAlgorithm) must remain private.


2.2 DeconvolutionParameters (Abstract Base)

Required properties as documented:

public abstract DeconvolutionType DeconvolutionType { get; protected set; }
public int MinAssumedChargeState { get; set; }
public int MaxAssumedChargeState { get; set; }
public Polarity Polarity { get; set; }
public AverageResidue AverageResidueModel { get; set; }

Obligation: All five of these properties must be present on the base class. The wiki shows AverageResidueModel as part of the base — any addition to the base class (e.g. ExpectedIsotopeSpacing, ToDecoyParameters()) is an extension of this contract, not a violation, but should be documented.


2.3 DeconvolutionAlgorithm (Abstract Base)

Required structure:

public abstract class DeconvolutionAlgorithm
{
    protected readonly AverageResidue AverageResidueModel;
    protected readonly DeconvolutionParameters DeconvolutionParameters;

    internal abstract IEnumerable<IsotopicEnvelope> Deconvolute(
        MzSpectrum spectrum,
        MzRange range);
}

Obligation: Deconvolute must be internal abstract. The two protected fields must be present.


2.4 IsotopicEnvelope (Result Type)

Required public interface:

public readonly List<(double mz, double intensity)> Peaks;
public double MonoisotopicMass { get; private set; }
internal double MostAbundantObservedIsotopicMass { get; private set; }
public readonly int Charge;
public readonly double TotalIntensity;
public readonly int PrecursorId;
public double Score { get; private set; }

Obligation: All of these members must exist. Score must be settable (privately). MostAbundantObservedIsotopicMass is internal only.


2.5 DeconvolutionType Enum

Documented values:

public enum DeconvolutionType
{
    ClassicDeconvolution,
    IsoDecDeconvolution,
    ExampleNewDeconvolutionTemplate
}

Obligation: These three values must exist. New algorithms require a new enum value and a corresponding case in CreateAlgorithm().


3. Averagine Models

Documented models:

Model Biomolecule Formula
Averagine Proteins/Peptides C4.9384 H7.7583 N1.3577 O1.4773 S0.0417
OxyriboAveragine RNA/Nucleotides C9.75 H12.25 N3.75 O7 P

Obligation: Both models must remain available. New models (e.g. DecoyAveragine) should be added to this table in a wiki update.


4. ClassicDeconvolutionParameters Contract

Required constructor and parameters:

new ClassicDeconvolutionParameters(
    minCharge: int,
    maxCharge: int,
    deconPpm: double,
    intensityRatio: double,
    polarity: Polarity = Polarity.Positive,
    averageResidueModel: new Averagine()  // optional
);

Required properties:

  • DeconvolutionTolerancePpm
  • IntensityRatioLimit

Obligation: These constructor parameters and property names must remain stable. The wiki example shows deconPpm and intensityRatio as the positional parameter names.


5. IsoDecDeconvolutionParameters Contract

Required user-accessible parameters (all must remain):

Parameter Type Default Description
PhaseRes int 8 Precision of encoding matrix
CssThreshold float 0.7 Min cosine similarity score
MatchTolerance float 5 Peak matching tolerance (ppm)
MaxShift int 3 Max allowed isotopic shift
MzWindow float[] [-1.05, 2.05] m/z window
KnockdownRounds int 5 Iterative refinement rounds
MinAreaCovered float 0.20 Min isotopic pattern coverage
DataThreshold float 0.05 Relative intensity threshold
ReportMultipleMonoisos bool true Report multiple monoisotopic peaks

Obligation: All nine parameters must remain with their documented names, types, and defaults.


6. Factory Pattern Contract

CreateAlgorithm() must map each DeconvolutionType to its algorithm:

private static DeconvolutionAlgorithm CreateAlgorithm(DeconvolutionParameters parameters)
{
    return parameters.DeconvolutionType switch
    {
        DeconvolutionType.ClassicDeconvolution => new ClassicDeconvolutionAlgorithm(parameters),
        DeconvolutionType.IsoDecDeconvolution => new IsoDecAlgorithm(parameters),
        _ => throw new MzLibException("DeconvolutionType not yet supported")
    };
}

Obligation: Every new DeconvolutionType value must have a corresponding case here, or the _ fallback will throw at runtime.


7. Extensibility Contract (New Algorithm Checklist)

The wiki explicitly defines the checklist for adding a new algorithm:

  • Add DeconvolutionType enum value
  • Create parameter class inheriting from DeconvolutionParameters
  • Implement algorithm class inheriting from DeconvolutionAlgorithm
  • Override Deconvolute() with algorithm logic
  • Update CreateAlgorithm() factory in Deconvoluter
  • Add unit tests for algorithm
  • Document parameters and use cases
  • Update the wiki with algorithm details

Obligation: All eight items must be completed for any new algorithm. The wiki update is explicitly listed as a requirement.


8. Parameter Classes Contract

Each parameter class must:

  • Inherit from DeconvolutionParameters
  • Set DeconvolutionType to the matching enum value
  • Call base(minCharge, maxCharge, polarity, averageResidueModel) in the constructor
  • Expose algorithm-specific parameters as public properties

9. Deconvolution Flow Contract

The documented steps are:

  1. Input validation — check spectrum type and range
  2. Algorithm selection — factory creates appropriate algorithm from parameters
  3. Charge state scanning — test charge states within specified range
  4. Pattern matching — compare observed peaks to theoretical isotopic distributions
  5. Scoring — evaluate quality of each isotopic envelope match
  6. Result generation — return scored isotopic envelopes

Obligation: NeutralMassSpectrum must be short-circuited before algorithm creation, returning peaks directly as single-peak envelopes.


10. Integration Contracts

Dependencies (must remain)

MassSpectrometry
  ↓ MzLibUtil
  ↓ Chemistry (ChemicalFormula in AverageResidue)

External Libraries (must remain available)

  • IsoDec: isodeclib.dll, isogenmass.dll
  • MathNet.Numerics: Statistical calculations
  • Intel MKL: libmmd.dll, svml_dispmd.dll

Downstream consumers (must not break)

  • Proteomics: Precursor mass determination from MS1
  • FlashLFQ: Quantification of isotopic envelopes
  • MetaMorpheus: Precursor refinement
  • mzLib applications: Any charge state determination use case

11. IsotopicEnvelope Filtering Contract

The wiki documents these as supported filtering patterns — they must continue to work:

// By score
envelopes.Where(e => e.Score > 100)

// By charge
envelopes.Where(e => e.Charge == 2)

// By mass range
envelopes.Where(e => e.MonoisotopicMass >= 1000 && e.MonoisotopicMass <= 5000)

// By intensity
envelopes.OrderByDescending(e => e.TotalIntensity).First()

Obligation: Score, Charge, MonoisotopicMass, and TotalIntensity must remain public readable properties/fields.


12. Range Handling Contract

// Must support explicit m/z range
var range = new MzRange(500, 1500);
Deconvoluter.Deconvolute(spectrum, deconParams, range);

// Must support isolation window
Deconvoluter.Deconvolute(scan, deconParams, scan.IsolationRange);

Obligation: The third rangeToGetPeaksFrom parameter must default to null (full spectrum) and accept any MzRange.


Summary — What the Wiki Does NOT Yet Document

These are gaps in the current wiki relative to the work in progress. Each represents an obligation to update the wiki before or alongside the PR merge:

Feature Status
FLASHDeconvolution algorithm Not documented
RealFLASHDeconvolution algorithm Not documented
DecoyAveragine model Not documented
ExpectedIsotopeSpacing on base class Not documented
ToDecoyParameters() abstract method Not documented
DeconvoluteWithDecoys() on Deconvoluter Not documented
DeconvolutionScorer generic scorer Not documented
Target-decoy FDR workflow Not documented

@nbollis nbollis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A step in the right direction but missed the mark. The intended revision was to allow us to statically register an EXE path and then the parameters does not need to store the path and the algorithm does not need to resolve at each decon call

range ??= spectrum.Range;
if (spectrum.Size == 0) return Enumerable.Empty<IsotopicEnvelope>();

string exePath = ResolveExePath(p.FLASHDeconvExePath);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still resolving EXE path at decon call :(

throw new FileNotFoundException($"mzML not found: {mzmlPath}", mzmlPath);

FLASHDeconvRunner effectiveRunner = runner ?? RunFLASHDeconvDefault;
string exePath = ResolveExePath(p.FLASHDeconvExePath);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here :(

  RealFLASHDeconvolutionAlgorithm no longer searches for FLASHDeconv. It
  now reads params.FLASHDeconvExePath directly and throws if it is null,
  empty, or points at a non-existent file. The algorithm has no knowledge
  of well-known install paths or the PATH environment variable.

  Path discovery lives on FlashDeconvExePathRegistry as a new public
  Resolve(string?) entry point (with a test-friendly overload that takes
  the well-known list and PATH-env as parameters). DefaultWellKnownPaths
  also moved here. Callers locate the exe once at startup -- e.g.
    string exe = FlashDeconvExePathRegistry.Resolve(globalSettingsPath);
    params.FLASHDeconvExePath = exe;
  -- and the algorithm just runs with what it was given.

  Tests: ResolveExePath_* renamed to Resolve_* and now exercise the
  registry's Resolve overload. Added Deconvolute_NullExePath_ThrowsMzLibException
  to pin the new contract. Removed Deconvolute_WithRegisteredPath_SkipsFilesystemValidation
  because the optimization it pinned (algorithm consulting the registry
  to skip File.Exists) no longer exists -- the algorithm always validates
  the explicit path it was handed.

  Addresses reviewer feedback that exe-path resolution does not belong
  inside a DeconvolutionAlgorithm.

@nbollis nbollis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool with registration. A few small details to look at

trishorts added 3 commits May 11, 2026 11:36
  FlashDeconvExePathRegistry exists to cache validated exe paths and skip
  the filesystem walk on repeat lookups, but every prior Section E test
  went through the internal three-argument Resolve overload (documented
  as "does NOT consult or update the cache"). The cache round-trip itself
  and Register's pre-warm contract were untested -- a regression that
  broke them would have passed.

  - Resolve_PublicMethod_SecondCallAfterFileDeleted_ReturnsCachedValue
    pins the Resolve(path) -> TryGet/CacheValidated round-trip by deleting
    the file between calls. Only a cache hit can produce the second result.
  - FlashDeconvExePathRegistry_RegisterThenDeleteFile_ResolveReturnsRegisteredValue
    pins that Register(path) seeds the cache under the explicit-path key
    (not the <default-search> sentinel) so Resolve(path) can return without
    touching disk afterwards.
  - Resolve_PublicMethod_RegisteredExplicitPath_DoesNotSatisfyDefaultSearch
    pins the inverse: an explicit-path Register MUST NOT hijack the
    default-search lookup -- the two cache keys stay disjoint.
  - Resolve_ExplicitPathExists_ReturnsItWithoutConsultingWellKnown and
    Resolve_ExplicitPathMissing_ThrowsFileNotFoundWithPathName directly
    exercise the internal Resolve overload's explicit-path branches, which
    were previously only hit indirectly through the algorithm's separate
    RequireExePath guard.

  Also added [SetUp]/[TearDown] that clears the static registry between
  tests so the new Count/cache-state assertions don't flake on execution
  order. Existing per-test Clear() calls left in place; they're redundant
  but harmless and document each test's expectations in isolation.
  Register only checked File.Exists, so any existing file -- including a
  .psmtsv or other unrelated file -- would register successfully and only
  fail later as an opaque Process.Start error on the first Deconvolute
  call.

  Add a filename-shape guard: the basename must start with "FLASHDeconv"
  (case-insensitive). Catches the .psmtsv mistake at registration time
  with a clear ArgumentException naming the bad filename. Permissive
  enough to accept version-suffixed binaries (FLASHDeconv-3.5.exe) and
  the Linux extension-less name.

  Test updates:
  - New: RegisterWrongFilename_ThrowsArgumentException pins the new check.
  - CreateFakeExeFile helper renamed its output from realflash_fake_<guid>.exe
    to FLASHDeconv_fake_<guid>.exe so existing Register-based tests still
    pass the validation.
  - RegisterMissingPath_ThrowsFileNotFound: renamed its nonexistent path
    to start with "FLASHDeconv" so it still exercises the File.Exists
    branch, not the new filename-shape branch.

  Addresses external review comment inline-3220263450 from @nbollis on PR smith-chem-wisc#1045.
@trishorts

Copy link
Copy Markdown
Contributor Author

PR #1045 adds DeconvolutionType.RealFLASHDeconvolution to the mzLib enum (confirmed: the enum now contains
ClassicDeconvolution, ExampleNewDeconvolutionTemplate, IsoDecDeconvolution, RealFLASHDeconvolution). MetaMorpheus's
switch handles the first three but not the fourth → the default: fires → every test that constructs a
DeconHostViewModel (directly, or transitively via DeconExplorationTabViewModel, or via a static [OneTimeSetUp] in
ChimeraGroupViewModelTests) crashes. The 56 TypeInitializationException failures are a knock-on from a single
static-init crash in ChimeraGroupViewModelTests.

What you need to fix in mzLib

Nothing. This isn't a mzLib bug.

  • mzLib's PR Real OpenMS FLASHDeconv using exe from provided installation path #1045 correctly added a new public enum value.
  • mzLib's own dispatch (Deconvoluter.CreateAlgorithm) is exhaustive.
  • MetaMorpheus's DeconHostViewModel switch is intentionally non-exhaustive with a fail-loud default — the comment on
    line 117 says so. The whole point is to force the MetaMorpheus maintainer to make a conscious decision when a new
    deconvolution type lands.

What MetaMorpheus needs (your fix is here): add a case DeconvolutionType.RealFLASHDeconvolution: arm to
DeconHostViewModel.cs:44-119. Two reasonable shapes: 1. All 114 failure stacks trace through DeconHostViewModel..ctor (PowerShell match across failures.json). It really is
one root cause, not a shared cluster hiding a second one. After you add the case arm, those 114 tests are the ones
that will turn green.
2. MetaMorpheus has zero references to FlashDeconvExePathRegistry, RealFLASHDeconvolution, or
FLASHDeconvExecutablePath anywhere in the tree. That means:
- MM never calls Register() today, so the filename-shape validation we tightened earlier this session can't bite it.
- No Deconvoluter.Deconvolute(...) call path in MM would dispatch to the new algorithm post-fix.
- There's no second wave of failures waiting to surface once you patch the switch.

So one switch arm in MetaMorpheus (case DeconvolutionType.RealFLASHDeconvolution: continue;) closes the entire
integration gap. The mzLib side of PR #1045 has nothing waiting to be repaired.

The only forward-looking note: when MM actually wants to expose RealFLASHDeconvolution to users, they'll need both (a)
a real GUI case arm and (b) a startup hook to call
FlashDeconvExePathRegistry.Resolve(GlobalSettings.FLASHDeconvExecutablePath) once and stash the result on params. But
that's a feature-add, not a bug fix — not needed to green the tests.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants