Update instrumentControl branch - #4
Open
avcarr2 wants to merge 314 commits into
Open
Conversation
* Reduces complexity of GetOneBasedScan from O(logN) to constant * Removed excess changes * Added a comment and removed extra line --------- Co-authored-by: Edwin Laboy <63374885+elaboy@users.noreply.github.com> Co-authored-by: trishorts <mshort@chem.wisc.edu>
…rackets (#726) * correct Within calculation * update unit tests * this is the spot * add space * New peptideWithSetMods output with mass shift in brackets * afds * nuthin * deleted path to hd * f * test essential sequence in peptideWithSetMods * fixed mod sig figs to six --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu>
* correct Within calculation * update unit tests * this is the spot * add space * New peptideWithSetMods output with mass shift in brackets * afds * nuthin * deleted path to hd * f * test essential sequence in peptideWithSetMods * fixed mod sig figs to six * no change just debugging * no chantge * tests fixed * remove unused code --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu>
* ini * // * Tests Passing * Cleaning * Deleted redundant files * Directory path fix * Tests Passing * tests passed * clean up * stream.Close() removed * better coverage * ready to go * Lock added to ensure only one thread can access msDataScans * Update ThermoRawFileReader.cs * fixes from comments in PR --------- Co-authored-by: Nic Bollis <nbollis@comcast.net> Co-authored-by: trishorts <mshort@chem.wisc.edu> Co-authored-by: Alexander-Sol <41119316+Alexander-Sol@users.noreply.github.com>
Co-authored-by: trishorts <mshort@chem.wisc.edu>
* Added in ability to parse TopPIC search result files * Added ability to write entire header * Added comments * Excluded Test class from code coverage * Adjusted some fields to match ours better * Handled a few edge cases * Added support for TopPIC version 1.5.3 * Added support for TopPIC version 1.5.3 * Fixed up operators --------- Co-authored-by: Nic Bollis <nbollis@wisc.edu> Co-authored-by: trishorts <mshort@chem.wisc.edu>
* correct Within calculation * update unit tests * this is the spot * add space * first move * psmFromTsv unit tests * moved library spectrum * empty unit test for library spectrum * m * library spectrum unit tests * lib spec unit tests * PSMTSV unit tests * add tests for variants and localized glycans * capitalization convention * read internal ions test * uncomment lines * moved fragmentation and library spectrum to new project Omics * Revert "moved fragmentation and library spectrum to new project Omics" This reverts commit d1bc75c. * someInterfaces * good midpont * omics classes and interfaces seem tobe working * move LibrarySpectrum class to Omics. Create SpectrumMatchFromTsvHeader interface in Omics * not working * Fixed up the PR * fix broken test * some unit tests * dhg * Expanded test coverage on file classes * new header and xlink psmtsv reader unit tests * space update * update nuspec for omics and added peptide folder to omics fragmentatkion * remove anyCPU library spectrum is child class of mzspectrum spectrum match from tsv now abstract class * remove empty constructro from mzspectrum --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu> Co-authored-by: nbollis <nbollis@comcast.net>
* correct Within calculation * update unit tests * this is the spot * add space * remove tab --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu>
* correct Within calculation * update unit tests * this is the spot * add space * first move * psmFromTsv unit tests * moved library spectrum * empty unit test for library spectrum * m * library spectrum unit tests * lib spec unit tests * PSMTSV unit tests * add tests for variants and localized glycans * capitalization convention * read internal ions test * uncomment lines * moved fragmentation and library spectrum to new project Omics * Revert "moved fragmentation and library spectrum to new project Omics" This reverts commit d1bc75c. * someInterfaces * good midpont * omics classes and interfaces seem tobe working * move LibrarySpectrum class to Omics. Create SpectrumMatchFromTsvHeader interface in Omics * not working * Fixed up the PR * fix broken test * some unit tests * dhg * Expanded test coverage on file classes * new header and xlink psmtsv reader unit tests * space update * update nuspec for omics and added peptide folder to omics fragmentatkion * Moved around most everything that wil need to be for Transcriptomics implementation * Made all tests pass * Moved a few methods out of PeptideWithSetModifications and into IBioPolymerWithSetMods * Moved methods from ProteolyticPeptide to LysisProduct * Marked RNase.tsv to copy always * Cleaned up the code quite a bit * Updated product class equalit members * Updated product class equalit members * This one method keeps fighting me * Removed AnyCPU * Added tests to ChemicalFormual operators * Update mzLib.sln * Updated Nuspec * Changed naming convention in Digestion Agent * Added comment to clarify base loss ions from 5' end Adjusted Chemical Formula subtraction when null - value * Added Comments to BioPolymerWithSetModsExtensions Made LysisProduct and abstract class and its constructor protected * Renamed LysisProduct to DigestionProduct Changed IDigestionParams.Enzyme to IDigestionParams.DigestionAgent * changed name on one method in DigestionProduct * Renamed get fixed mods method in DigestionProduct --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu> Co-authored-by: trishorts <mshort@chem.wisc.edu>
* correct Within calculation * update unit tests * this is the spot * add space * remove tab * remove extra tab from peptides and proteins output of flashlfq --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu>
* Create CODE_OF_CONDUCT.md * Update CODE_OF_CONDUCT.md
* Create CONTRIBUTING.md * Update CONTRIBUTING.md * Update CONTRIBUTING.md --------- Co-authored-by: Anthony <anthony.cesnik@gmail.com>
* Changed behavior of masked members in Digestionparams * Added unit test * Added unit test
…icaiton (#757) * Made Required Changes to interfaces * Added clone method to digestion params * Added nucleotide and Rnase * Excluded test classes from code coverage * Removed unused using directives * Expanded test coverage * Added novel terminus parmeter to the clone method * Added reviewer suggested comments * Added comments from reviewer response * Added MRS genename comment
* correct Within calculation * update unit tests * this is the spot * add space * update proteasesTsv --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu>
* Changed Deconvoluter to have a static call. This enables parallelization in MM * adjusted deconvoluter.deconvolute for msdatascan to be able to switch and set specific parameters removed deconvoluter extensions class * Expanded test coverage by handling all switch cases * Added ContainsMajority to DoubleRange * Added filtering to msdatascan get isolated masses and charges * Update mzLib.nuspec * Expanded test coverage in msdatascan
* Predict Retention Time function added * breaking up mbr function, cont'd * Decoy search is working * Removed decoy toggle from GetIsotopicEnvelopes * About to start MbrScorer refactor * Refactor of MBR is working succesfully * Deleted unused code * Deleted unused fields and references to PearsonCorrelation in isotopicEnvelope class and CheckEnvelope method * Deleted unused using statements in FlashLFQ engine * Commented + removed unused variables for PredicteRetentionTime method * Added comments, removed unused using statements * Deleted unused variables --------- Co-authored-by: Nic Bollis <nbollis@comcast.net>
* Edited the MatchedFramgentIon Equals method * Report mbr predicted rt * Fixed Equals and GetHashCode method for MatchedFragmentIon --------- Co-authored-by: Nic Bollis <nbollis@comcast.net> Co-authored-by: trishorts <mshort@chem.wisc.edu>
* handled edge case with digestion product having a null parent * Added more test cases * Adjusted conditionals to get full coverage
* correct Within calculation * update unit tests * this is the spot * add space * unused using * vnc * add unit test --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu>
* Update dotnet.yml Added coverlet dependency "System.Text.Json" to the github actions workflow * Update dotnet.yml * Update dotnet.yml Rollback coverlet to previous version
…faster (#765) * correct Within calculation * update unit tests * this is the spot * add space * unused using * vnc * add unit test * unit test updates * some renaming * code quality * same * k * l * fh * add new bool for theo peaks and fix one group of unit tests * all tests fixed * make SpectralEntropy only work under conditions where it should * delete empty space --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu>
* Added msfragger psm to supported file type and get recognition from path working * All Tests Pass * Expanded test coverage to read/write * Expanded Test Coverage * One last bit of test coverage * All fragger peptides and proteins load and counts correct * Added support for fragger peptide and protein files --------- Co-authored-by: Edwin Laboy <63374885+elaboy@users.noreply.github.com>
* Update dotnet.yml Attempting to fix issue with codecov * Update dotnet.yml * Update dotnet.yml --------- Co-authored-by: trishorts <mshort@chem.wisc.edu>
…ng a pep with set mods (#773) Co-authored-by: Edwin Laboy <63374885+elaboy@users.noreply.github.com> Co-authored-by: trishorts <mshort@chem.wisc.edu>
* Allow multiple custom fragmentations * fixed neutral mass * Added comments to the dissociation type collection classes
* Added optional arg to FlashLFQ that allows users to select which peptides will be quantified/reported * added some comments * increasing test coverage * Increased test coverage * Renamed, added comment --------- Co-authored-by: Edwin Laboy <63374885+elaboy@users.noreply.github.com>
* correct Within calculation * update unit tests * this is the spot * add space * unused using * vnc * revert IntensityPairs method from pr 765 --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu>
…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>
* correct Within calculation * update unit tests * this is the spot * msl foundation * msl writer * reader * some more tests * more tests * benchmark complete * custom neutral loss * compression * compression complete * streaming * koina connections * proteoforms who would have thought * proteoform scoring * msl merger * python and rust readers for msl * msl test coverage * fix test * coverage gaps * polish prompts * koina * mslmerger * chore: ignore BenchmarkDotNet.Artifacts output directory * a little help from copilot * only happens in github * write streaming * wrap up lurking bugs * remove unused class * more unused code * test mslstructs * eliminate more unused code * spectrallibrary tests * mslreader coverage * write msp or msl depends on file extension in koina * msp loaded for bear as msl * tests for stripped modification * null sequence guard * be gentle * normalize retention time of incoming spectra to existing library * variable naming consistency * nics review respnse * internal review * review fixes * fix: qualify SpectralLibrary type in merged Prosit2020 test (namespace shadows type) * perf(msl): defer the sequence/charge index so LoadIndexOnly does no fragment I/O MslIndex.Build eagerly populated the sequence/charge dictionary, and because a peptide's modified sequence is not in the compact index struct it fetched each one through the entry loader. In index-only mode that loader reads the entry's fragment block from disk — so building the dictionary read EVERY fragment in the library, defeating the point of LoadIndexOnly (a mass/RT window workflow that never does a sequence lookup paid a full-library fragment read up front). Make _bySeqCharge nullable and build it lazily: MslIndex's ctor/Build take a deferSeqChargeIndex flag, LoadIndexOnly passes it, and TryGetBySequenceCharge builds the dictionary on first use (EnsureSeqChargeIndex, locked, once). The elution-group map is unaffected — it needs only the compact struct. Full-load mode is unchanged (its loader is in-memory, so eager build stays cheap). On an 11-database parallel search this cut per-database index-only load from ~0.7-4.0 s to ~0.2-1.6 s and removed the reader's dominant cost. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(chronologer): batched RetentionTimePredictorEquivalents for many-peptide prediction The base PredictRetentionTimeEquivalents fans out over PredictCore, but every PredictCore call runs the model on a batch of one and serializes on the model lock — so predicting N peptides was N locked batch-1 forward passes, the bulk of .msl library build time. Override it in ChronologerRetentionTimePredictor: format/encode all peptides in parallel (pure CPU, no model access), then run the model in large batched forward passes (one lock acquisition per chunk). The model is in eval mode, so BatchNorm uses running statistics and each peptide's prediction is independent of the batch — results are byte-identical to the per-peptide path. Building a ~9,400-entry library dropped from ~46 s to ~3.7 s (~12x), with identical stored iRT (verified min/max/non-null match). This makes large library builds (1000s of databases) practical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(msl): cover the deferred sequence/charge index (LoadIndexOnly lazy build) Adds deferred-vs-eager parity tests for MslIndex: a deferred index (deferSeqChargeIndex: true, as LoadIndexOnly uses) returns the same TryGetBySequenceCharge results as an eagerly-built one once the lazy EnsureSeqChargeIndex fires, reports misses for absent sequence/charge, and is consistent across repeated lookups. Locks the behaviour of the defer-the-seq/charge-index change that lets LoadIndexOnly open a library without fragment I/O. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(msl): skip NaN-m/z prefix in MslIndex m/z queries float.CompareTo orders NaN before every finite value, so Array.Sort places NaN-m/z entries at index 0. QueryMzRange started its upward scan there and stopped at the first PrecursorMz <= mzHigh failure (NaN <= x is false), so a single NaN-m/z precursor returned an empty result for the ENTIRE library -- silent total data loss for any library containing e.g. a peptide with the ambiguous residue X (undefined mass -> NaN m/z). Record _firstFiniteMz after sorting and start the lower-bound binary search there, so m/z-window queries run on the monotonic finite range and simply exclude the unmatchable NaN-m/z entries. Add regression tests for a leading-NaN entry and an all-NaN index. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(development): untrack committed BenchmarkDotNet.Artifacts results These six generated benchmark reports (csv/html/github-md) were committed before the **/BenchmarkDotNet.Artifacts/ .gitignore rule, so they stayed tracked and produce noisy diffs/merge conflicts on every benchmark run. Remove them from source control; the existing ignore rule keeps them out. Addresses PR #1036 review comment (committed benchmark artifacts). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(msl): assert internal-ion secondary annotation survives ReadFragmentIon 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 #1036 review comment (internal-ion annotation completeness). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(msl): clarify proteoform scorer reports library reference peaks 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 #1036 review comment (library vs experimental match values). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(speclib): restore space indentation, dropping tab-reindent churn 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> * chore: drop unrelated test changes from the .msl PR 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> * chore(msl): remove agent-residue comments and internal "Prompt N" refs 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 #1036 review (agent residue / mechanical assembly). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(msl-tests): rename PromptN fixtures and strip prompt-process 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 #1036 review (agent residue / mechanical assembly). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: restore mzLib.nuspec version to 1.0.574 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 #1036 review comment ("Change Nuspec back"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(rt): drop batched Chronologer PredictRetentionTimeEquivalents 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 #1036 review ("Is this method needed? ... use the existing method"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(chronologer): dispose forward() intermediates to stop native-heap 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> * refactor(chronologer): drop env-var CUDA switch from the batched predictor 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 #1077. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(msl): clarify MslFragmentIon vs MatchedFragmentIon + round-trip 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> * chore(msl): remove Python and Rust external MSL reader implementations 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> --------- Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
--- updated-dependencies: - dependency-name: OpenMcdf dependency-version: 3.1.4 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
) Chronologer.forward() allocates ~30 intermediate tensors per prediction (residual clones, conv/norm/relu outputs) that were never explicitly disposed. They were reclaimed by the GC finalizer thread, which calls libtorch's native allocator concurrently with the main inference thread and corrupts the native heap (STATUS_HEAP_CORRUPTION, 0xC0000374). The race is volume-dependent, so it stays hidden for small inputs and only surfaces in high-volume runs such as spectral-library builds. Wrap each PredictCore inference in a TorchSharp dispose scope so every intermediate is freed deterministically on the calling thread. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Nic Bollis <nbollis@comcast.net>
…verflow (#1079) * fix(msl): reject >32,767 fragments/entry on write instead of silent overflow MslPrecursorRecord.FragmentCount is int16, so an entry with more than 32,767 fragment ions silently wrapped to a negative count on write and threw OverflowException only later on read. Validate on write and throw a clear ArgumentException naming the entry and the limit. Surfaced while verifying the .msl manuscript's "unbounded fragments per entry" claim; reachable for ~250+ residue proteoforms annotated with internal ions. A true fix (widen FragmentCount to int32) is a versioned format change; this is the safe non-breaking guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TM8gpAxcjWYschz3yEjixZ * test(msl): cover >32,767 fragment write-guard on both paths Add four NUnit tests for the FragmentCount int16 overflow guard: the in-memory (Write) and streaming (WriteStreaming) paths each reject a 32,768-fragment entry with an ArgumentException naming the entry and the 32,767 limit, and the 32,767 boundary still writes with no wrap (locks the comparison as '>' rather than '>='). Previously the throw branch was unhit, so whole-file coverage masked ~29% patch coverage on the new guard; this lifts it to 100%. Ref: #1079 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Feature (#1069) * add new protease subtilisin * feat: add subtilisin|p protease and unit test Add subtilisin|p to the embedded proteases.tsv with full cleavage specificity and proline-inhibition motifs (N[P]|, S[P]|, L[P]|, K[P]|, I[P]|, D[P]|, Y[P]|, V[P]|, G[P]|, F[P]|, T[P]|, E[P]|, Q[P]|, A[P]|, R[P]|). Add TestSubtilisinP_DigestsCorrectlyAndRespectsProlineRestriction to ProteinDigestionTests to verify: - subtilisin|p is present in the embedded protease dictionary with CleavageSpecificity.Full - All expected cleavage sites fire on a proline-free sequence (ANKTIDE) - The K[P]| proline-inhibition rule is respected (AKPIDE keeps KP intact) * Accept newer-TopFD column names in Ms1Feature PR 1064 ships an Ms1Feature reader that recognises only the older FlashDeconv / TopFD-v1.6.2 _ms1.feature schema (Sample_ID, ID, Time_begin, Time_end, Minimum_charge_state, Maximum_charge_state, Minimum_fraction_id, Maximum_fraction_id). Newer TopFD output keeps the same _ms1.feature extension but uses different column names (File_name, Fraction_ID, Feature_ID, Min_time, Max_time, Min_charge, Max_charge), plus extras like Envelope_num and EC_score. Format detection picks Ms1FeatureFile by extension; CsvHelper then throws because none of the expected [Name(...)] columns are present. Discovered while integrating PR 1064's FromFileDeconvolutionParameters into MetaMorpheus and pointing it at a real TopFD .ms1.feature from a top-down yeast run -- the file parsed by hand looks identical in shape to the old schema, just relabelled. Fix is column-name aliases on the existing record, plus [Optional] on fields that the newer schema omits entirely. Downstream GetSingleChargeFeatures() reads only Mass, ChargeStateMin/Max, RetentionTimeBegin/End, and IntensityApex -- all aliased to a column present in both schemas, so the join algorithm behaves identically regardless of which producer wrote the file. Per-field summary: Sample_ID -> [Optional] (newer TopFD has File_name instead; type-incompatible -- int vs string path -- and not used downstream) ID -> alias "Feature_ID" + [Optional] Time_begin -> alias "Min_time" Time_end -> alias "Max_time" Minimum_charge_state -> alias "Min_charge" Maximum_charge_state -> alias "Max_charge" Minimum_fraction_id -> [Optional] (newer TopFD has a single Fraction_ID column, not a min/max pair; not used downstream so alias would add no value) Maximum_fraction_id -> [Optional] No tests added in this commit -- a follow-up should drop a newer-TopFD _ms1.feature sample into Test/FileReadingTests/ExternalFileTypes/ and extend the existing Ms1FeatureFile read-roundtrip tests to cover both schemas. The current Ms1Feature_FlashDeconvOpenMs3.0.0_ms1.feature Ms1Feature_TopFDv1.6.2_ms1.feature fixtures keep passing because the existing [Name(...)] heads remain the first entry in every alias list. * Drops a 4-row fixture (Ms1Feature_TopFDvLatest_ms1.feature) captured from real TopFD output that uses the File_name / Fraction_ID / Feature_ID / Min_time / Max_time / Min_charge / Max_charge schema, and wires it into the existing TestMsFeature parameterised tests: * TestFeaturesLoadAndCountIsCorrect gains a TestCase asserting the fixture loads four features end-to-end via FileReader. * TestTopFDLatestMs1FeatureFirstAndLastAreCorrect locks the per- field mapping for both the aliased columns (Time_begin/Min_time, Minimum_charge_state/Min_charge, etc.) and the [Optional] fields absent from the newer schema (SampleId, FractionIdMin/Max default to 0). Covers the single-charge edge case (Min_charge == Max_charge == 1) in the last record. * TestTopFDLatestMs1GetSingleChargeFeatureFunctions confirms charge- range expansion is identical across the two TopFD schemas: a 6-14 range yields 9 envelopes; a 1-1 range yields exactly one; GetMs1Features() flattens to 9 + 12 + 10 + 1 = 32 across the four fixture features. * TestMs1FeatureReadWrite gains the new fixture as a TestCase. The writer emits the older-schema headers (newer columns aren't on the record), so the round-trip converts schema-newer -> schema- older + Optional defaults. Comment in the test explains why that is correct: every field downstream consumers actually read survives the round-trip; the columns that don't are exactly the ones marked [Optional] and unused. All 17 TestMsFeature tests pass, as do the 148 tests covering the related Ms1Feature / FromFile / SupportedFileExtensions / DinosaurTsv surface area. * Promote consensus mass-tracing types out of Development Move the post-decon consensus pipeline (per-charge trace grouping, median off-by-one correction, cross-charge feature stitching) from Development.Deconvolution.ConsensusTracing into a new production namespace MassSpectrometry.Deconvolution.Consensus. The types had been parked in Development as the research arc explored their design (see NOTES.md, phases 1-13); promotion was flagged as the next move once the design stabilised. New files under mzLib/MassSpectrometry/Deconvolution/Consensus/: * MassTrace -- per-scan envelope list at one charge, anchored to the first envelope's mass. * MassTraceBuilder -- greedy charge-locked grouper, gap-tolerant within a configurable mass tolerance. * CorrectedEnvelope -- one envelope inside a corrected trace, carrying both original and post-correction mass plus a WasCorrected flag. * CorrectedTrace -- a MassTrace after weighted-median off-by-one correction. * TraceCorrector -- the corrector itself, with uniform/intensity/ scorer-weighted variants via an EnvelopeWeight delegate. * MassFeature -- a cross-charge feature: a group of CorrectedTrace entries whose consensus masses agree within a ppm tolerance and whose RT windows overlap. * MassFeatureBuilder -- the union-find sweep that produces them. No behaviour change. The classes, methods, and signatures are identical to the research-namespace versions; only their namespace and file locations changed (each type now lives in its own file). Doc comments updated to drop references to phase numbers and to the "Development" location. The research scaffolding in Development.Deconvolution.ConsensusTracing (Phase*.cs fixtures, MakeConsensusSnips, LargeTestDataLocator) stays on the consensus-tracing branch; it will be updated to consume the new namespace when that branch is rebased on top of this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add Ms1FeatureFile.FromMassFeatures factory Lets the consensus-tracing pipeline persist its cross-charge feature list as a FLASHDeconv-style _ms1.feature file -- the existing wire format any Ms1FeatureFile consumer (notably MetaMorpheus's FromFileDeconvolutionParameters) already understands. The consensus-to-search bridge then needs no new file format and no new MetaMorpheus surface area. Two pieces: * MassFeature.ToMs1Feature(sequentialId, sampleId = 0, fractionId = 0) extension method (under Readers, because the return type is the Readers-namespace Ms1Feature record). Maps: Mass <- MassFeature.ConsensusMass Intensity <- MassFeature.SummedIntensity RetentionTimeBegin <- MassFeature.RTStart RetentionTimeEnd <- MassFeature.RTEnd RetentionTimeApex <- RT of the highest-intensity envelope on the highest-summed-intensity constituent trace ("apex of the dominant charge state at its most intense scan") IntensityApex <- intensity of that same envelope ChargeStateMin/Max <- min/max of MassFeature.Charges SampleId / FractionId <- callers' choice (defaults 0) * Static Ms1FeatureFile.FromMassFeatures(IEnumerable<MassFeature>, sampleId, fractionId, software) factory. Builds the records via ToMs1Feature, populates a fresh Ms1FeatureFile instance, and returns it. The caller invokes .WriteResults(path) on the returned file to actually emit bytes -- no new IO path, the existing reader's writer is reused. Software label defaults to FLASHDeconv because that's the canonical schema the writer emits; the [Name(...)] alias machinery from PR #1064 + schema-aliases PR means newer-TopFD readers also see the output as valid. No filter applied: every MassFeature becomes one row. The original question surfaced four filtering options (all, multi-charge only, score-thresholded, all + side artifact); per the design pin "all features, no filter" is the right default. Downstream consumers (MetaMorpheus's precursor HashSet + FDR machinery) handle the noise. The writer is decoupled from the producer: callers pass a sequence of finalised MassFeature objects; the factory doesn't care where they came from. Future producers (mass-trace results from non- Classic algorithms, etc.) can use the same path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Cover Ms1FeatureFile.FromMassFeatures with a round-trip test Five NUnit cases under Test\FileReadingTests\ExternalFileReading\TestMs1FeatureFromMassFeature.cs: * ToMs1Feature_SingletonFeature_MapsAllFields -- one trace, one envelope. Locks the mapping: Mass, Intensity, RT begin/end/apex, apex intensity, charge bounds, fraction id all come from Finalise()-derived MassFeature fields. * ToMs1Feature_MultiChargeFeature_ApexIsDominantTraceMaxEnvelope -- two-charge feature with five envelopes. Confirms apex is "highest- intensity envelope on the highest-summed-intensity constituent trace", and that cross-charge consensus mass is the intensity- weighted mean of per-trace consensus masses. * ToMs1Feature_SampleIdAndFractionIdHonoured -- callers can override Sample_ID and Fraction_ID for multi-file/multi-fraction outputs. * FromMassFeatures_WriteThenRead_AllFieldsSurvive -- three features built in-memory, written via WriteResults, re-read with FileReader.ReadFile<Ms1FeatureFile>, every downstream-consumed field compared to the original ToMs1Feature output. Catches any serialisation drift between writer and reader on the new path. * FromMassFeatures_AssignsSequentialIdsFromZero -- producer's internal MassFeature.Id is ignored; written rows get 0..N-1. Keeps the output file's IDs dense and stable even when the upstream pipeline has filtered features and left holes. Fix discovered while writing the second-to-last test: the original FromMassFeatures factory used `file.Results.Add(...)` in a loop, which goes through ResultFile<T>.Results' lazy-load getter. The getter calls LoadResults() whenever the backing list is empty; for an in-memory- constructed Ms1FeatureFile with no FilePath, LoadResults throws ArgumentException on the empty path. Reshaped the factory to build the records up front, then assign Results once via the setter (which bypasses the lazy-load). Inline comment explains why. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add [Explicit] end-to-end driver from mzML to _ms1.feature Demonstrates the consensus-to-search loop the rest of this PR was built for: load an mzML, run Classic decon per MS1 scan with top-down parameters (charge 1..60), build mass traces with Pass-B grouping (loose 1.5 Da to cohort off-by-one twins), apply weighted-median correction, stitch cross-charge features at 10 ppm, write the resulting MassFeature list as a FLASHDeconv-style _ms1.feature via the new Ms1FeatureFile.FromMassFeatures factory, and round-trip read the written file as a sanity check. Lives as an NUnit [Explicit] test because it consumes a 165 MB local mzML at a hard-coded path that's not part of the repo or CI: E:\TestData\MetaMorpheus\05-26-17_B7A_yeast_td_fract7_rep1.mzML Assume.That on File.Exists keeps it silently skipped on machines without the data. Run with: dotnet test --filter FullyQualifiedName~TestConsensusToMs1FeatureEndToEnd On the yeast top-down test file (4837 scans total, ~2700 MS1) the driver writes a 24 MB _ms1.feature with 266,373 features. That file, fed to MetaMorpheus's FromFile precursor path (#2650), yields a Cal -> GPTMD -> Search pipeline that converges to a tighter precursor mass tolerance (1.6 ppm) than the equivalent classic-only baseline (2.4 ppm) and finds 907 PSMs at 1% FDR vs 785 -- a 15.5% increase in identifications from consensus precursors alone, before any additive combination with the classic source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Parameterise consensus driver via TestCaseSource Lets a single dotnet test invocation run the consensus pipeline over multiple raw/mzML files in succession. Each case captures its own source path, output path, and decon charge ceiling -- bottom-up uses MaxCharge=12 (matching MetaMorpheus's standard BU default), top-down would use 60. Currently shipping two bottom-up HEK293 Velos raws as the parameterised test cases: * 20100609_Velos1_TaGe_SA_293_3.raw (5163 MS1 scans, 76878 features after correction, 7756 multi-charge -- 8.9 s wall time) * 20100609_Velos1_TaGe_SA_293_4.raw (5111 MS1 scans, 74173 features, 7318 multi-charge -- 5.7 s) Each test case still gates on File.Exists via Assume.That; cases for machines without the data skip silently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Normalise feature-file RT to minutes when loading FlashDeconv and TopFD both canonically emit _ms1.feature files with RetentionTime columns in seconds (e.g. Time_begin = 2787 for a 46-min LC run). MetaMorpheus and mzLib's MsDataScan.RetentionTime are always in minutes (mzML / Thermo convention). FromFileDeconvolutionAlgorithm compared the file's RT directly against the scan's RT, so on any real FlashDeconv or TopFD output the seconds-vs-minutes mismatch produced zero overlapping windows -- and zero PSMs at search time. Caught while running a Cal -> GPTMD -> Search pipeline over a 20-mzML Jurkat top-down dataset using its FlashDeconv companion _ms1.feature files. Classic-only produced 23,636 PSMs at 1% FDR; FromFile-only produced 0 with no error or warning -- silent failure mode. Fix: in the file-path FromFileDeconvolutionParameters constructor, detect seconds-as-loaded via a max-RetentionTimeEnd > 500 heuristic (no realistic LC run exceeds 8 hours = 500 min) and divide all RT fields by 60. Files already in minutes pass through unchanged. The internal pre-loaded-features constructor (test seam) is unaffected so callers supplying explicit synthetic units still get raw values. Verification: re-running the same Jurkat pipeline after the fix yields 23,976 PSMs in FromFile-only mode and 39,700 PSMs in additive Both mode (vs Classic-only's 23,636) -- +68% PSMs / +68% proteoforms over the Classic baseline on this dataset. Tests: * Existing EndToEnd_FilePathCtor_ResolvesExpectedChargeState updated to use MS2 RT in minutes (39.85) instead of seconds (2390.0) -- the fixtures are FlashDeconv / TopFD with seconds-RT and the fix now normalises them at load, so the test had to match the new units. * New FromFileDeconvolutionParameters_FileWithRtInSeconds_NormalisesToMinutes writes a synthetic seconds-RT file, loads via the file-path ctor, asserts every per-charge feature's RT lands in the expected minutes range (~39.83 / ~40.17), and pairs against an in-minutes MS2 to confirm overlap is found. * New FromFileDeconvolutionParameters_FileWithRtInMinutes_LeavesUnchanged sister test: file already in minutes (max RT < 500) is NOT double-converted. 31/31 tests in TestFromFileDeconvolution + TestMs1FeatureFromMassFeature pass after the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(decon): split resolvable co-grouped species in trace correction TraceCorrector now derives its off-by-one window from per-trace mass scatter (3*sigma via MAD, capped below half the isotope spacing) instead of a fixed +/-0.05 Da. An envelope at a resolvable, non-isotope offset (e.g. a deamidated form at +0.98402 Da, which the old fixed window snapped to the unmodified mass) is split into its own CorrectedTrace and surfaced as a distinct feature; when scatter is too large to resolve it from the 1.00335 Da isotope spacing it conservatively merges. Correct() now returns List<CorrectedTrace>. Bottom-up search needs this -- deamidation detection is a requirement there. Also in this hardening pass: - MassFeatureBuilder sorts components by (mass, charge, RT) before assigning IDs, so feature IDs and written _ms1.feature row order are deterministic instead of depending on Dictionary enumeration. - Guard clauses with clear messages on Finalise (empty traces), ToMs1Feature (un-finalised feature), and BuildTraces (scan/envelope length mismatch). - FromFileDeconvolutionParameters exposes RetentionTimeNormalizedFromSeconds and warns when the seconds->minutes heuristic fires, rather than silently mutating RT. - Unit + performance tests for the consensus engines (0 -> 100% line coverage on the promoted types) covering the behaviours above. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(decon): add TD/BU feature generators + per-feature decon scoring [Explicit], local-data drivers used to benchmark the consensus pipeline: JurkatMs1FeatureGenerator and VelosMs1FeatureGenerator regenerate the _ms1.feature files for the Jurkat top-down (charge 1..60) and HEK293 Velos bottom-up (charge 1..12) datasets via the new deamidation-split pipeline. ConsensusFeatureScoring computes the generic per-envelope deconvolution score (DeconvolutionScorer.ScoreEnvelope: averagine cosine, ppm error, peak completeness, intensity-ratio consistency) for every Classic envelope and aggregates it per feature into a sidecar TSV aligned to the _ms1.feature row order. This gives a uniform, search-independent feature-quality value for a computed noise-filter cutoff -- scored at generation time because the consensus pipeline keeps only mass+intensity per envelope and drops the peaks the scorer needs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(readers): preserve gapped charge sets on _ms1.feature round-trip Hardening fixes to the consensus MassFeature -> _ms1.feature path surfaced by code review. None were observed in normal use, but each is a real edge-case correctness or crash issue: - Write one row per contiguous charge run so a gapped set (e.g. {10,12,15}, when intermediate charges fall below the score cutoff) no longer reads back as the fabricated full range 10..15. Adds ToMs1Features. - FromMassFeatures: empty input writes a valid header-only file instead of crashing; also fixes a LoadResults infinite recursion when reading a zero-row file (the Results getter re-entered LoadResults on an empty set). - Ignore non-finite RetentionTimeEnd when sniffing seconds-vs-minutes, so a single NaN row can't flip an in-minutes file into a /60 conversion. - Default the produced file's Software to TopFD to match how it is re-detected on reload (the writer always emits Apex_intensity); fix the misleading "FLASHDeconv canonical" doc. - Document that per-charge Intensity comes from Apex_intensity, not the summed Intensity column. Adds NUnit coverage for each; tidies now-stale FLASHDeconv comments. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(consensus): use Constants.C13MinusC12 for isotope spacing TraceCorrector defined its own IsotopeSpacingDa = 1.00335, a less-precise copy of the canonical Chemistry.Constants.C13MinusC12 (1.00335483810). Alias the shared constant so the C12/C13 spacing has one source of truth; the 4.8e-6 Da change is far inside the off-by-one detection window, so results are unchanged. Addresses review feedback on #1069. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(readers): only lazy-load Results when the file exists ResultFile<T>.Results lazy-loaded whenever _results was empty, which crashed for a factory-built file with an empty FilePath (e.g. a zero-feature Ms1FeatureFile.FromMassFeatures). Guarding the load with File.Exists(FilePath) lets an in-memory file return its set records directly, removing the need for Ms1FeatureFile's parallel _factoryRecords store so the records have a single source of truth. Addresses review feedback on #1069. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(consensus): add XIC-vs-MassTrace grouping comparison harness Two [Explicit] drivers (local Jurkat top-down data, not CI) that back the decision to keep MassTraceBuilder over reusing the existing GetAllXics: TestXicVsMassTraceComparison times both groupers head-to-head on the same deconvoluted envelopes (wall-clock, allocations, trace counts, grouping agreement); JurkatMs1FeatureGeneratorViaXic emits XIC-grouped _ms1.feature files through the identical downstream so end-to-end proteoform yield can be compared (GetAllXics measured -18% vs MassTraceBuilder). Refs #1069. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add consensus pipeline hierarchy overview to MassFeature doc comment Defines how IsotopicEnvelope, MassTrace, CorrectedTrace, and MassFeature nest and which builder produces each level, per review request. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(rt): compare duplicate-peptide predictions within tolerance, not bit-exactly PredictRetentionTimeEquivalents_DuplicatePeptides_ReturnsSameValueForEach asserted exact equality via Distinct().Count() == 1. The Chronologer batch path runs the duplicate peptides as identical rows in a single libtorch forward pass, whose CPU conv/matmul kernels are not bit-reproducible across thread counts / CI runners, so identical rows can differ in the last float32 bits (intermittent CI failure: distinct == 2). The values are correct to float32 precision — the rest of this suite already pins batch agreement with .Within(1e-4). Assert every duplicate prediction is non-null and equal to the first within 1e-4, matching the suite's existing tolerance. Production code unchanged; the real heap-corruption non-determinism was already fixed in #1075. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011QVxqemPqC9BjvYtVg6aKY --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: MICHAEL SHORTREED <mrshortreed@wisc.edu>
…#1078) * feat(decon): expose most-abundant and average observed masses on envelopes Add proton-corrected MostAbundantObservedMass and AverageObservedMass to IsotopicEnvelope, plus an EnvelopeResolution flag, and GetMostAbundantOffset/ GetAverageOffset helpers on AverageResidue. These let downstream search select proteoform candidates by the experimentally most-detectable peak (the most abundant isotopologue, or the centroid for unresolved high-mass species) instead of the often-undetectable monoisotopic peak, avoiding off-by-N precursor mass errors. The existing un-proton-corrected MostAbundantObservedIsotopicMass is retained for compatibility. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(decon): address PR #1078 review — drop GetMostAbundantOffset, clarify mass docs Remove the GetMostAbundantOffset(mass) convenience from AverageResidue: it was just GetDiffToMonoisotopic(GetMostIntenseMassIndex(mass)), so consumers now compose the existing public methods directly (reviewer: "isn't this the same as diff to monoisotopic?"). Clarify the docs distinguishing MostAbundantObservedMass (proton- corrected neutral mass) from the existing MostAbundantObservedIsotopicMass (m/z x |charge|, not proton-corrected) (reviewer: "isn't this handled by the existing field?"). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(decon): compute observed envelope masses on access MostAbundantObservedMass and AverageObservedMass are fully derivable from the readonly Peaks and Charge, so make them computed get-only properties instead of fields set in every constructor. Removes ComputeObservedMasses() and its three call sites, eliminating the constructor boilerplate and the redundant no-op on the single-peak file-read path. Legacy MostAbundantObservedIsotopicMass is unchanged. Addresses review comments from PR #1078 (nbollis). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(decon): harden most-abundant/average observed-mass coverage - Bound the average offset from above so a sort-order/sign regression that inflates GetAverageOffset is caught (was lower-bound only). - Tighten the centroid-vs-most-abundant assertion from 0.5 Da slack to 1e-6 now that the perfect-envelope centroid is confirmed >= apex. - Add an AverageObservedMass == mono + GetAverageOffset round-trip, mirroring the existing most-abundant round-trip. - Cover the 5-arg deconvolution constructor via a BuildPerfectPeaks helper shared with the 6-arg path. Addresses test-quality findings from the PR #1078 review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(deconvolution): tighten AverageObservedMass bounds and cover zero-intensity fallback Address minor test-quality findings from the PR #1078 review. - AverageObservedMass_IsBetweenMonoAndAboveMostAbundant now asserts an upper bound (centroid <= heaviest peak's neutral mass), so the "between" contract the test name advertises is actually enforced rather than only lower-bounded. - Add AverageObservedMass_ZeroTotalIntensity_FallsBackToMostIntensePeakMass covering the totalIntensity == 0 centroid fallback (asserts a real mass, not NaN from divide-by-zero). The empty-peaks guard and GetAverageOffset's zero-total-intensity fallback are unreachable through the public API (the constructor's MaxBy throws on an empty value-type sequence; averagine intensities are never all-zero), so those defensive branches are left documented rather than exercised via reflection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(envelope): scope PR to resolved most-abundant mass; public field + -1 sentinel Keep this PR focused on the resolved most-abundant-peak feature and move the isotopically-unresolved (centroid/average) machinery to a separate PR. - Expose MostAbundantObservedIsotopicMass as public (was internal) so consumers can read the observed most-abundant peak, and document the terminology (most-abundant vs monoisotopic vs average/centroid) explicitly on the members. - Constructors with no observed isotopic envelope (the neutral-mass-from-file path) now set MostAbundantObservedIsotopicMass = -1 (sentinel for "no most-abundant peak available") instead of a misleading 0. - Derive MostAbundantObservedMass from that single field (proton-corrected; differs by exactly charge*ProtonMass) so the two can never disagree, and it propagates the -1 sentinel. Also removes the per-access peak rescan. - Move out to the unresolved PR: AverageObservedMass, the EnvelopeResolution enum, Resolution/SetResolution, AverageResidue.GetAverageOffset, and their tests. The resolved search uses the pre-existing averagine most-abundant offset (GetDiffToMonoisotopic(GetMostIntenseMassIndex)). Deconvolution suite green (667 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011QVxqemPqC9BjvYtVg6aKY * refactor(envelope): rename MostAbundantObservedMass -> MostAbundantObservedNeutralMass Per review feedback: the property is the proton-corrected neutral mass of the most-abundant peak, so name it explicitly to distinguish it from the m/z×|charge| MostAbundantObservedIsotopicMass. Adjust the doc comments and tests to the new name; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011QVxqemPqC9BjvYtVg6aKY --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Nic Bollis <nbollis@comcast.net>
#1080) The mzIdentML reader ships three structurally-identical, XSD-generated DTO trees (mzIdentML110/111/120.Generated). Hand-written per-property tests only touched 111, leaving 110 and 120 near-zero and dragging down overall coverage. Add one parameterized test that reflects over every concrete generated type in each namespace and round-trips every property getter/setter. Concrete subclasses inherit the abstract base accessors (AbstractContactType, IdentifiableType, AbstractParamType), so those are exercised too. Result is 100% line and branch coverage on all three files from a single test. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add MzLibUtil.CvParam, a serialization-agnostic record modeling the PSI controlled-vocabulary parameter (cvParam) shared across mzML/mzIdentML/mzTab and web APIs such as PRIDE Archive. Full PSI field set including the optional unit triplet; init-only properties; no JSON/XML serializer dependency (deserializers map wire names by case-insensitive matching). Consumers key on Accession (the stable term identifier), not Name. Covered by CvParamTests (9 tests). Claude-Session: https://claude.ai/code/session_01CaQVBed3CG6HuF8Nkpdzoo Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: trishorts <mshort@chem.wisc.edu>
…e, don't block (#1090) Adopt the shared external-service test convention (matching the MetaMorpheus side): live web-service tests carry [Category("ExternalService")] and run in a dedicated, non-blocking CI job so a third-party outage never blocks a PR, and self-classify an outage as Skipped (with a reason on the log) rather than Failed. - Add ExternalServiceTestHelper (RunAsync / EnsureReachable / ThrowIfUnavailable), identical to the MetaMorpheus copy. - Add KoinaLiveTestFixture: a [OneTimeSetUp] probe of Koina's readiness endpoint that skips the whole fixture when Koina is unreachable. The four live Koina fixtures now inherit it and gain [Category("ExternalService")] alongside [Category("Koina")]. - dotnet.yml: * build/test run now filters Category!=ExternalService (supersedes Category!=Koina) * new non-blocking external-service-tests job runs Category=ExternalService * the MetaMorpheus integration run now filters Category!=ExternalService, so a UniProt (or other) outage in MetaMorpheus's suite no longer reddens every mzLib PR - this integration job clones MetaMorpheus master and runs its whole test suite. Note for maintainers: leave external-service-tests OUT of branch-protection required checks. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(pepxml): cover generated pepXML_v120 DTOs via reflection pepXML_v120.cs is a single ~7k-line XSD-generated DTO tree (64 classes, ~470 properties, 112 optional-value "...Specified" flags) with no test coverage. Hand-writing per-property set/get tests is what historically tanks coverage on these generated files, so follow the same reflection approach just landed for the mzIdentML DTOs (#1080): - walk every concrete type in pepXML.Generated and round-trip each property's getter/setter; - round-trip every generated enum value through its name (the XmlSerializer path); - serialize/deserialize a populated msms_pipeline_analysis tree so the Xml* attributes across the whole tree are validated end to end. Brings pepXML_v120.cs to 100% line coverage (2400/2400). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(pepxml): assert round-tripped fields and harden reflection probes - MsmsPipelineAnalysis round-trip now asserts precursor_neutral_mass, mod_aminoacid_mass.mass, specificity.sense, and search_database.type, so the documented float/shape checks actually validate (a dropped field would previously have passed green). - CreateValue probes enums with a non-default member so a no-op setter no longer satisfies the set/get assertion. - Clarify the enum test comment: it walks CLR member names, not the [XmlEnum] serializer mapping. Addresses 3 findings from the PR #1082 review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(readers): add MostAbundantMassDiffPpm psmtsv column header Registers the optional "Most Abundant Mass Diff (ppm)" column in the canonical psmtsv header set (SpectrumMatchFromTsvHeader), alongside MassDiffPpm. This is the most-abundant-mode analogue of the monoisotopic Mass Diff (ppm): the observed most-abundant isotopic peak vs the candidate's theoretical averagine-apex mass. Previously the header string was defined locally in MetaMorpheus's PsmTsvWriter; moving it here makes mzLib the single source of truth for psmtsv column names, so MetaMorpheus (PR #2671) can reference the constant instead of duplicating the literal, and any mzLib reader can map the column by name. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(readers): parse MostAbundantMassDiffPpm so the column round-trips Mirrors MassDiffPpm on the read side so the optional "Most Abundant Mass Diff (ppm)" column round-trips through the psmtsv reader, not just the header registry: - SpectrumMatchFromTsv.MostAbundantMassDiffPpm string property (null when the column is absent, matching how MassDiffPpm is read). - ParseHeader registers the column; GetOptionalValue reads it only when present, so monoisotopic-mode files (no column) are unaffected and nothing throws. - Disambiguation constructor copies/splits it null-safely (via ?.) for ambiguous matches, matching the MassDiffPpm treatment. Test: MostAbundantMassDiffPpm_OptionalColumn_ReadsAndDisambiguates covers absent->null, present-value read, non-ambiguous copy, and both disambiguation branches (single- and multi-segment monoisotopic mass). FileReadingTests 609/609 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* laxy load and toml ignore
* fix test
* revert(FromFileDecon): remove TOML/Nett surface from PR
The TOML round-trip motivation is being dropped from this PR. It added a
Nett dependency, [TomlIgnore] attributes, and three TOML tests for a
feature (transparent deserialize of FromFileDeconvolutionParameters) that
Nett 0.15.0 cannot actually support — the type has no parameterless ctor
and Nett does not honor the public FilePath setter as a creation path.
Without the ability to round-trip, the TOML surface adds a dependency
without a real consumer.
This commit reverts all of that:
- Readers.csproj: drop Nett 0.15.0 PackageReference
- mzLib.nuspec: drop Nett 0.15.0 from both <group> dependency lists
- FromFileDeconvolutionParameters.cs: drop 'using Nett;' and the four
[TomlIgnore] attributes (on _featureCacheLock, _featureCache, Features,
RetentionTimeNormalizedFromSeconds). DeconvolutionType was already
reverted to no attribute. The lazy-load plumbing and FilePath property
are unchanged; the I/O-free Equals/Hash refactor that lives on the
same file is reverted in this commit and re-applied in the follow-up
commit (the test contract change is conceptually separate from the
TOML removal).
The TOML round-trip tests were already removed from the working tree
before this commit, so they don't appear here.
The PR is now a pure lazy-load + FilePath refactor with no TOML surface.
* test(FromFileDecon): lock lazy-load + I/O-free equality + FilePath cache contracts
Addresses the five review comments on the lazy-load PR:
1. FromFileDeconvolutionParameters.cs: EqualProperties and AddHashCodes
now key on FilePath alone. The previous code read Features.Count
(lines 145, 152), which forced LoadFeaturesFromFile on every Equals
or GetHashCode call. The base class still contributes the other
config fields (MinAssumedChargeState, MaxAssumedChargeState, Polarity,
ExpectedIsotopeSpacing, UseGenericScore, AverageResidueModel), so
equality is still a complete config-only identity. GetHashCode and
Equals are now I/O-free and cannot throw, even on an instance whose
FilePath points at a missing or non-feature file.
2. New tests lock the I/O-free contract:
- FromFileDeconvolutionParameters_GetHashCode_OnUnloadedInstance_DoesNotLoadFile
constructs from a missing path and asserts GetHashCode() does not
throw and returns a non-trivial value.
- FromFileDeconvolutionParameters_Equals_OnUnloadedInstances_DoesNotLoadFile
compares two instances built from the same path without ever
touching .Features.
3. New tests lock the FilePath setter / cache-invalidation contract:
- FromFileDeconvolutionParameters_SetFilePath_InvalidatesCache:
materializes the cache from path A, assigns FilePath = pathB,
accesses .Features again, asserts the second access reflects path
B (cache was dropped, the new file was re-read).
- FromFileDeconvolutionParameters_RepeatedFeaturesAccess_LoadsOnce:
three sequential .Features accesses return the same list reference
(double-checked locking, no re-read).
4. The lazy-load claim is now actually tested:
- FromFileDeconvolutionParameters_NonFeatureFile_ThrowsMzLibException
is rewritten so the constructor and the .Features access are no
longer in the same try. The new shape proves the ctor did not
throw (no I/O) and the MzLibException is provably from .Features.
- FromFileDeconvolutionParameters_NonExistentFile_ConstructorDoesNotThrow_LoadDoes
is added with the same shape on a non-existent path (Assume.That
guard on File.Exists) and a relaxed Throws.Exception assertion
(FileReader can surface FileNotFoundException or MzLibException
depending on extension).
5. TestDeconvolutionParametersEqualityAndClone updated to the new
equality contract: FromFile_Equal_DifferentFeatureCount_AreNotEqual
becomes FromFile_Equal_DifferentFeatureCount_AreEqualWhenNoFilePath
(no FilePath -> feature count no longer participates in equality),
and FromFile_Equal_DifferentFilePaths_AreNotEqual is added to lock
the FilePath-distinguishes-them half.
Full test suite passes (4675 tests). No production behavior change other
than the equality/hashing refactor; no public API change.
* Refactor Koina models: neutral loss support, output extraction by name, unrestricted CE
- Add ParsedFragmentAnnotation record with neutral loss parsing
- Extract API outputs by name instead of positional index
- Fix intensity variable bug (was using m/z instead of intensities)
- Support neutral loss mass calculation in fragment ion matching
- Add CreateUnimodConverterAcceptAll for models accepting any modification
- Make KoinaModelBase members protected for subclass extensibility
- Remove collision energy restrictions on Prosit2020IntensityHCD
- Move DetectabilityModel to AbstractClasses namespace
* Refactor Koina model layer: deduplicate batching, add switch exhaustiveness, fix validation
- Add BuildBatchedRequest helper + InputField record to KoinaModelBase,
eliminating ~770 lines of duplicated dictionary construction across 37 models
- Remove Guid.NewGuid() from batch IDs (simpler counter-based IDs);
server ignores the id field for deduplication
- Add 'default: throw' to all ParameterHandlingMode switches for
future-proofing against new enum values
- Replace positional Outputs[0] lookup in CollisionalCrossSectionModel
with name-based "ccs" output resolution
- Move crosslink beta-null check from AsyncThrottledPredictor loop into
ValidateModelSpecificInputs for consistent validation
- Remove stale Outputs.Count != 3 guards (ExtractOutputs validates by name)
- Add virtual NumberOfPredictedFragmentIons to FragmentIntensityModel base;
override in 19 concrete models for polymorphic access
* Fix BaseFragmentIdentifier string arithmetic, PFly2024 null-forgiving, thread-safety docs, RT ToArray
- Fix BaseFragmentIdentifier computed property: use IndexOf('-') instead of
string-length arithmetic that broke for multi-character neutral loss formulas
(e.g. H3PO4 would include the trailing dash in the base identifier)
- Add missing null-forgiving operator on ValidatedFullSequence in
PFly2024FineTuned.ToBatchedRequests (consistent with all other models)
- Document thread-safety caveat on FragmentIntensityModel,
CollisionalCrossSectionModel, CrosslinkFragmentIntensityModel,
and DetectabilityModel (previously only on RetentionTimeModel)
- Remove unnecessary .ToArray() in RetentionTimeModel.ResponseToPredictions
(List<string> already implements IReadOnlyList<string>)
* Fix Contains('+') filter, TryGetValue indexer, CCS realignment + revert charge guard
- Remove Contains('+') pre-filter in GenerateLibrarySpectraFromPredictions
that silently dropped Altimeter (^ delimiter) and UniSpec annotations.
Parse failures are now caught by try/catch around ParseFragmentAnnotation
in the second loop, matching the ResponseToPredictions pattern.
- Replace tpLookup indexer (KeyNotFoundException risk) with TryGetValue
+ continue, consistent with ResponseToPredictions at line 458.
- Add ParameterWarning fallback in CollisionalCrossSectionModel realignment
(was SequenceWarning-only, unlike the other two model types).
- Change BaseFragmentIdentifier LastIndexOf('-') to IndexOf('-') to match
the dash-finding logic in ParseFragmentAnnotation.
- Revert the charge <= 0 guard in CollisionalCrossSectionModel.ValidateModelSpecificInputs
and add a TODO to revisit what empty AllowedPrecursorCharges means.
* Implement null/empty semantics for allowed parameter collections
Phase 1 — Validation logic change:
- Change AllowedCollisionEnergies, AllowedInstrumentTypes, AllowedFragmentationTypes
base property types from HashSet<T> to HashSet<T>? with default null
- null = parameter not applicable to this model (skip validation entirely)
- empty HashSet = parameter IS required but any value accepted
- populated HashSet = parameter IS required AND value must be in set
- Split the combined 'missing required params' check into per-parameter checks
with individual error messages
Phase 2 — Model migration:
- Models that send collision_energies to Koina keep empty HashSet (required, any value):
All Prosit fragment models (except CID), UniSpec, AlphaPeptDeep, XLCMS2, XLNMS2
- Models that don't send collision_energies change to null (not applicable):
Prosit2020IntensityCID, all 7 Ms2Pip models, Prosit2023IntensityXLCMS3
- Altimeter keeps its populated range (already correct)
- XLCMS2 and XLNMS2 add explicit AllowedCollisionEnergies = {} override
(previously inherited now-changed base default)
* Phase 4: Document null/empty/populated semantics on Allowed* properties
- Add XML doc comments to AllowedPrecursorCharges, AllowedCollisionEnergies,
AllowedInstrumentTypes, and AllowedFragmentationTypes in
FragmentIntensityModel documenting the three-tier convention
- Add same docs to AllowedPrecursorCharges and AllowedCollisionEnergies
in CrosslinkFragmentIntensityModel
- Update AllowedUnimodIds doc in KoinaModelBase to clarify it belongs to
the converter layer (not parameter validation) with empty = accept none
- Update TODO in CollisionalCrossSectionModel to reference the new
null/empty convention and note AllowedPrecursorCharges isn't yet migrated
* Fix review feedback
- Fix MapToInputFullSequence desynchronization bug in FragmentIntensityModel.cs
- Correct Prosit2024IntensityXLNMS2 fragment ion count from 174 to 348
- Fix Prosit2020IntensityTMT TryCleanSequence return value
* Fix Koina model validation, MS2PIP Immuno HCD length, CMS3 null CE, and batch type mismatches
- FragmentIntensityModel: require non-empty constraint sets before rejecting null params
- CrosslinkModelTests: handle null AllowedCollisionEnergies for CMS3
- Ms2PipImmunoHCD: correct MaxPeptideLength from 15 to 30
- CollisionalCrossSectionModel: add non-physical charge guard
- CrosslinkFragmentIntensityModel: fix sequence validation warning propagation
- CollisionEnergy: cast to float across all model impls (int? -> float)
- UniSpec: case-insensitive instrument type matching
- Ms2PipCIDTMT: N-terminal TMT/iTRAQ label validation
- Crosslink benchmark: generate independent beta peptides
- Prosit2024IntensityXLNMS2: correct fragment ion count to 348
* Review updates: fix parameter validation, neutral loss parsing, benchmark/test bugs
* Fix FragmentIntensityModel validation tests defaulting untested constraints to empty sets
The TestFragmentIntensityModel constructor defaulted unspecified constraints
(AllowedCollisionEnergies, AllowedInstrumentTypes, AllowedFragmentationTypes)
to empty HashSets. Per the validation logic in FragmentIntensityModel.cs:
- null = skip validation (not applicable)
- empty set = parameter IS required but any value accepted
This caused 4 tests to fail because empty sets made untested constraints
required, while the test inputs provided null for those fields.
Fix: default to null instead of empty sets, so only explicitly-passed
constraints are validated.
* Koina: fix annotation/instrument/detectability handling, share HttpClient, add coverage
Audit follow-ups for the Koina prediction client:
- FragmentIntensityModel: route the MapToInputFullSequence branch through the
overridable ParseFragmentAnnotation and compute neutral-loss-aware m/z, matching
GenerateLibrarySpectraFromPredictions. Fixes a crash on non-'+' annotations
(Altimeter '^', UniSpec internal ions) and wrong m/z for neutral-loss ions.
- HTTP: back with a single process-wide HttpClient; per-request CancellationToken
timeout replaces the per-session client to avoid socket exhaustion under repeated
Predict() calls. Updated the five AsyncThrottledPredictor call sites.
- CollisionalCrossSectionModel: make AllowedPrecursorCharges nullable to follow the
null/empty/populated constraint convention; the charge >= 1 physical guard still
applies. Removes the standing TODO.
- DetectabilityModel: map ValidatedFullSequence from requestInputs rather than the
full ModelInputs list, so the reported sequence is correct when inputs are filtered.
- Prosit_2025_intensity_lac: send instrument types uppercased and accept them
case-insensitively. Koina's Prosit_Preprocess_instrument_types matches uppercase
and silently defaults unknown casing to LUMOS, so lowercase astral/eclipse were
silently mispredicted.
Tests (no network; run on every PR build):
- New unit fixtures for crosslink, CCS, detectability, fragment-intensity, and HTTP
covering TryCleanSequence, ValidateModelSpecificInputs, ResponseToPredictions,
the all-invalid realignment path, GenerateLibrarySpectraFromPredictions, and the
MapToInputFullSequence/neutral-loss parsing.
- Moved crosslink client-side validation tests out of the [Category("Koina")] fixture
so they count toward coverage.
PredictionClients line coverage ~49% -> ~66%; HTTP 0 -> 100%,
CrosslinkFragmentIntensityModel ~5% -> ~89%.
* minor previous test fix (one line)
* codecov
* more codecov
* Address Koina model review feedback
- Remove unused FragmentIonMappingMode from the crosslink model hierarchy
- Guard equal annotation/mz/intensity output array lengths in ResponseToPredictions
- Label library spectra with the sequence the masses were built from
- Add offline tests for XLNMS2 and TMT validation, TMT fragmentation type, and library labeling
* Address automated review: deadlock-safe Predict, bounded HTTP, model name discovery test
- Wrap CCS/Crosslink/RetentionTime/Detectability Predict in lock + Task.Run to avoid the documented single-threaded SynchronizationContext deadlock
- Require a CancellationToken on HTTP requests and add a coarse backstop client timeout
- Add deadlock regression tests and a live ModelName-resolves-to-Koina-endpoint test
* Koina: carry neutral loss on generated library spectrum fragments
* Koina: drop unused baseId out-param from Altimeter neutral-loss parser
---------
Co-authored-by: Nic Bollis <nbollis@comcast.net>
* chore(readers): upgrade Thermo CommonCore RawFileReader to 8.0.37 (net8) Replace the netstandard2.0 5.0.0.7 Thermo CommonCore binaries with the native .NET 8 8.0.37 build - Thermo's current, recommended target. Brings newer instrument/scan-filter support, memory-mapped raw I/O, and aligns the reader with mzLib's net8.0 target. The CommonCore API surface mzLib uses is unchanged (verified against Thermo's own net8 example and the test suite). Pin OpenMcdf back to the 2.x line (2.3.1): the RawFileReader binary is compiled against OpenMcdf 2.3.0 and 3.x is an API-incompatible rewrite that breaks .raw reading. GHSA-5qwm-7pvp-w988 (fixed in 3.1.4) is a DoS on crafted/untrusted CFB files and does not apply - we only read trusted local instrument files. Overlaps #1089. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(readers): guard Thermo CommonCore 8.x upgrade with compat tests Add TestRawFileReaderVersionCompat: a deployed-version canary plus invariant sweeps that exercise every CommonCore API surface mzLib's ThermoRawFileReader depends on - filter enums (MS order/polarity/mass analyzer), centroid and PreferredMasses spectrum extraction, trailer-extra parsing, scan-event reactions, and static/dynamic read parity - so any behavioral regression from this or a future reader bump fails loudly instead of silently corrupting data. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Nic Bollis <nbollis@comcast.net>
* feat(usefulproteomicsdatabases): add PRIDE Archive REST client
Add PrideArchiveClient, an async IDisposable client for the PRIDE Archive REST
API (v3). GetProjectFilesAsync(accession) returns the complete, auto-paged
manifest of a project's files as PrideArchiveFile objects, whose controlled-
vocabulary fields use MzLibUtil.CvParam. Newtonsoft deserialization with
NullValueHandling.Ignore; blank input throws ArgumentException, transport/HTTP
failures throw HttpRequestException, and a MaxPages cap guards against a server
that ignores paging. Unknown/empty projects return an empty list, never null.
Offline tests via a stub HttpMessageHandler (13 tests) run in the required
unit-test job. The live canary carries [Category("ExternalService")] and routes
through ExternalServiceTestHelper.RunAsync, so a PRIDE outage skips (not fails)
in the dedicated non-blocking external-service job.
Adds a Newtonsoft.Json 13.0.4 reference to UsefulProteomicsDatabases (matching
the version already used by PredictionClients).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CaQVBed3CG6HuF8Nkpdzoo
* feat(usefulproteomicsdatabases): thread CancellationToken through GetProjectFilesAsync
Address the actionable items from the PR #1088 review (nbollis):
- GetProjectFilesAsync accepts an optional CancellationToken, checks it
per page, and passes it to GetAsync / ReadAsStringAsync so a long
pagination sequence can be cancelled cooperatively.
- seal PrideArchiveClient (it owns a single HttpClient and is not a base
type), which is the intended resolution of the non-virtual-Dispose note.
- Add <inheritdoc/> to Dispose().
- Tests: double Dispose is idempotent; an injected HttpClient survives the
client's Dispose and stays usable; an injected HttpClient with no
BaseAddress gets the PRIDE default; pageSize validation now covers a
negative value; MaxPages cap is exercised at edge values 0 and 1.
The remaining review findings (Newtonsoft coupling, IHttpClientFactory,
IJsonSerializer/IPrideArchiveClient abstractions, thread-safe dispose,
User-Agent) were declined as inconsistent with mzLib conventions and
resolved on the PR.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016y1PW7eLKHQMHw7Ymp9p9p
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ication handling (L0) (#1084) * test(variants): add data-driven sequence-variant corpus (L0 foundation) The nut: an executable spec for how mzLib *should* read, expand, and digest proteins that carry sequence variants and modifications together — one of the most bug-prone corners in the library. Each case is a tiny protein described as one row in a table; the test builds its XML, runs the real LoadProteinXML -> variant-expansion -> top-down Digest pipeline, and compares the resulting proteoforms to a hand-written expected answer. Expected answers are derived from the intended biology, never captured from current code, so the corpus catches bugs instead of enshrining them. Details: - cases.tsv = one row per case (base sequence, mods, variants, protease, caps, extensible opts tail); expected/<id>.txt = the intended forms, one per line in canonical "least-modified-first" order, in mzLib's real full-sequence notation. - VariantCorpusTests asserts count + membership + run-twice determinism. - L0 foundation (substitutions + PTMs, top-down, non-binding caps): identity, single mod, single substitution, sub-x-mod chemical-legality drop (3 forms not 4), out-of-range pruning. All five green against master. - Later layers extend the same table: indels + source-dependent encoding, processing, bottom-up digestion, multi-variant/order, VCF depth, round-trip. Test-only; no production code touched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(variants): add ProteinDbWriter round-trip over the corpus Adds Corpus_Node_RoundTrip: serializes the consensus protein through the real ProteinDbWriter and re-reads it before digestion, so the corpus now exercises encode+decode (invariant 6 / #1083) as well as read+digest. The writer never sits upstream of the ground truth -- the input XML is still the hand-authored, bible-grounded one, and the round-trip reuses the same expected/<id>.txt oracle. A writer that drops a variant or a variant-adjacent mod fails here while Corpus_Node stays green. Green across L0 (F00-F04); becomes the tripwire for variant-borne-mod and indel serialization at L1. Addresses review feedback (@nbollis): use ProteinDbWriter in the harness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(variants): bind cases.tsv columns by header name, not position LoadRows parsed the spec table by magic index (c[7], int.Parse(c[8])...), which obscures the columns and silently breaks on any reorder or added column. Build the name->index map from the header once and construct Row with named fields (S(base), I(expected_count)); fail loudly listing any missing required column. cases.tsv is a deliberately growing table, so header-binding is the robust reading. Addresses review feedback (@nbollis): read the table into the typed object in a way that documents the columns. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(variants): store corpus as inline CorpusCase records, not tsv+txt Replace the external cases.tsv + expected/*.txt (and their csproj copy-to-output directives) with a CorpusCase record yielded inline from GetCases(), matching the house style in Transcriptomics/TestDigestion.cs. Input spec and expected forms now sit together on one compiler-checked, IDE-navigable record; no runtime file I/O, no file-not-found fragility. ExpectedCount is kept alongside ExpectedForms as an independent over/under-generation guard (asserted equal), mirroring RnaDigestionTestCase keeping both DigestionProductCount and Sequences. Verdict/Reason ride along as fields, preserving the spec documentation the table used to carry. README updated to describe the record schema. Both families green (10/10). Addresses review feedback (@nbollis): encode the cases into a class. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AppVeyor was retired in the 2022 CI migration; its badge has shown a stale failing status ever since. Repoint to the live "Build and Test" workflow (dotnet.yml) so the badge reflects real master status.
* Fix IndexOutOfRangeException on profile Bruker data with MS/MS trimming GetSpectraData indexed profileMzs[^0] (one past the end) when building the upper bound for WindowModeHelper.Run, so any profile-mode Bruker .d file loaded with a FilteringParams whose ApplyTrimmingToMsMs is true threw IndexOutOfRangeException. The centroid (line) branch already used the correct [^1]. Use [^1] and guard on Length > 1 to match that branch. This surfaced downstream in MetaMorpheus, whose MyFileManager always builds a FilteringParams (except for LowCID), crashing every profile-mode regular-TOF file on load; centroid files were unaffected. Add a parametrized TestBruker case that loads the centroid, profile, and profile+centroid fixtures with ApplyTrimmingToMsMs = true. The existing tests never passed a FilteringParams, so the trimming branch was uncovered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Ship Bruker baf2sql native deps in mzLib NuGet lib root Move baf2sql_c.dll and its VC++ 2012 runtime dependencies (vcomp110, msvcp110, msvcr110) from Readers/Bruker/ to the Readers project root so they copy to the output root, and switch BrukerFileReader's DllImports to the bare baf2sql_c name. This lets the native library resolve when mzLib is consumed as a NuGet package (e.g. MetaMorpheus), where lib files are flattened to the app root rather than a Bruker/ subfolder. Add baf2sql_c.dll and vcomp110.dll to the nuspec lib roots (both target frameworks); previously they were missing entirely, so .baf reading failed from the packaged library. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix Bruker .baf reader yielding empty database search results, write isolation Width and fixed trimming * Added additional tests --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(variants): read UniProt-native deletions (empty <variation>) The ProteinXmlEntry reader admitted a sequence-variant feature only when <variation> was non-empty, so it silently dropped every UniProt-native deletion (encoded as <original>X</original><variation/>, which is how UniProt writes a deletion and is not ours to change). Admit any feature that encodes a real change -- not both <original> and <variation> empty -- so deletions become SequenceVariations alongside substitutions/insertions. Corpus (L1 deletions, bible-derived oracle, red-before-fix): add nodes D00-D08 and MD00-MD02 -- single/multi-base deletions, deletions before/on/ after a PTM residue (phospho re-anchors/drops/stays), out-of-range and straddle-end pruning, the VCF depth cutoff (minAlleleDepth) pass/fail, and pairs of separate deletions (capped at 2). Harness gains BEGIN/END, empty VAR (deletion), SRC=vcf (GT/AD/DP), and a per-case MinAlleleDepth. Fix a latent fixture error the drop had masked: in seqvartests.xml the YPSE deletion declared begin=960, but YPSE sits at 961-964 (base is NYPSE at 960-964), violating SeqVarXmlTest's invariant that OriginalSequence equals the residues at [begin,end]. Corrected begin 960->961. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GbKQ6fMLGsmiPAHPN3W6gz * test(variants): give deletion round-trip teeth on read-but-unapplied variants; isolate D06 end>len Corpus_Node_RoundTrip previously asserted only the digested proteoform set, which for consensus-only nodes equals the base sequence whether or not the writer preserved the SequenceVariation. Add AssertConsensusVariationsRoundTrip comparing the consensus SequenceVariations (position + original + variant) before write vs after re-read, so a writer that drops a read-but-unapplied variant now fails the round-trip (real teeth on D08, the depth-filtered node whose variant is retained on the consensus). Out-of-range nodes (D05/D06) are pruned from the consensus at load, so both sides are legitimately empty. Retune D06 to OP=E BEGIN=7 END=8 so end>len is the sole reason for pruning (the original now matches residues[begin,end] in range), cleanly isolating the PruneOutOfRangeSequenceVariants end-beyond-length branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(variants): pin proteoform-level application of the native C-terminal YPSE deletion SeqVarXmlTest only asserts that P40467's UniProt-native YPSE deletion (empty <variation/> at 961-964, admitted by the #1095 reader fix) is READ with the right coordinates. Add SeqVar_NativeCTerminalDeletion_YPSE_AppliesAndShortensProtein to assert it is APPLIED correctly: taking the deletion exactly as read from the fixture and applying it in isolation yields a proteoform that ends ...YLPLN (length - 4), not ...YLPLNYPSE, with the applied variation carrying original "YPSE", empty variant, and post-collapse coordinates 961..960. A bug that read the native deletion yet failed to apply it would pass SeqVarXmlTest and fail here. Scoped to the deletions tranche; P40467's substitutions/insertions are left for their own tranches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…1099) * feat(usefulproteomicsdatabases): fetch PRIDE spectra by USI via PROXI Adds spectrum access to the PRIDE client (capability after the merged manifest MVP #1088 and file download #1093). Given a USI, GetSpectrumAsync returns the peaks as an mzLib MzSpectrum; GetProxiSpectrumAsync returns the raw PROXI object, keeping the controlled-vocabulary attributes (charge, precursor m/z, ms level, scan number, instrument). - PrideProxiSpectrum DTO (status/usi/mzs/intensities/attributes), attributes reuse MzLibUtil.CvParam like PrideArchiveFile. - PrideArchiveExtensions.ToMzSpectrum: pure, offline peaks->MzSpectrum transform; defensively sorts a copy by ascending m/z (MzSpectrum assumes sorted, does not sort itself) and rejects a mzs/intensities length mismatch. - PROXI lives under a different path root than the v3 archive base, so the request uses an absolute URI (verified live: resultType=full returns a 1-element JSON array; unknown USI 404s, malformed USI 400s -> HttpRequestException). - Adds a MassSpectrometry project reference (no dependency cycle). - Tests: 16 offline (fixture-based) + live canary [Category(ExternalService)] +[Category(Pride)] routed through ExternalServiceTestHelper (skip-on-outage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DZiz2D6dfQ55WetZpJSey2 * refactor(usefulproteomicsdatabases): PrideProxiSpectrum is an MzSpectrum; add ToMsDataScan Addresses review feedback on #1099. PrideProxiSpectrum now derives from MzSpectrum, so a fetched PROXI spectrum can be handed straight to anything in mzLib that takes an MzSpectrum and the ToMzSpectrum conversion step is gone. On top of that, ToMsDataScan reads the PROXI controlled-vocabulary attributes into a full MsDataScan, which an MzSpectrum alone cannot carry. - PrideProxiSpectrum : MzSpectrum. The peaks must reach the base constructor (MzSpectrum has no parameterless ctor and keeps XArray/YArray private to set), so the wire fields arrive through a [JsonConstructor] rather than being assigned by the deserializer afterwards; the DTO is immutable as a result. Sorts ascending on construction (MzSpectrum assumes sorted and does not sort itself) and still rejects a mzs/intensities length mismatch -- verified that the MzLibException surfaces unwrapped through Newtonsoft. - PrideArchiveExtensions.ToMzSpectrum removed; ToMsDataScan added, mapping scan number, ms level, polarity, retention time, centroid-ness, precursor m/z and charge, isolation window, injection time and filter string. Accession set verified against a live PRIDE response. Fields PROXI does not state are left Unknown/null rather than guessed: PROXI names the instrument, not the analyzer, and carries no dissociation term (it is only inside the vendor filter string, which is preserved verbatim as ScanFilter). Retention time is normalized to minutes, mzLib's convention, per unit term when present and as seconds when absent, which is what PRIDE serves. - ToMsDataScan lives beside the DTO rather than reusing Readers.Mzml's CV dictionaries: Readers references UsefulProteomicsDatabases, not the reverse, and all three of those dictionaries miss on PRIDE's payload anyway (it sends polarity as MS:1000465 + value, and no analyzer or dissociation term). - GetSpectrumAsync kept as the simple-case API, now returning the spectrum itself narrowed to MzSpectrum. - Tests: 22 offline, incl. the full realistic attribute set, the unstated-field defaults, both polarity/centroid spellings, retention-time units, and the ragged-wire contract. Live canary extended to assert ToMsDataScan against the real PRIDE payload. All 81 offline PRIDE tests + 2 live canaries green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(readers): convert MSFragger retention time from seconds to minutes IQuantifiableRecord.RetentionTime is documented as minutes and every consumer treats it that way, but MsFraggerPsm passed the tool's "Retention" column through unconverted — and MSFragger writes seconds. FlashLFQ then assigned it to ms2RetentionTimeInMinutes and built a +/-2 MINUTE peak-finding window around it, so a peptide eluting at 60 minutes was searched for at 1 minute. Quantification collapsed toward zero instead of failing, which is the most damaging way for this to be wrong. Fixes #1115. The raw column keeps a property of its own, now named for its unit (RetentionTimeInSeconds), so the file still round-trips on write and anyone reading the tool's value verbatim still can. The interface member converts. That mirrors what the spectra readers already do at their own boundary — BrukerFileReader, MsAlign, Mgf and Mzml all normalise to minutes — so both halves of the library finally agree on the unit. The bug was pinned by a test. TestIdentificationAdapter asserted Ms2RetentionTimeInMinutes == 1.9398, which is the seconds value sitting in a property named for minutes; TestMsFraggerResultFiles asserted the same number on the record. Both are corrected, and two new tests pin the contract itself: that the interface value is the raw one over 60, and that every retention time in the fixture is under a minute — five scans about a second apart on an LTQ Orbitrap Velos, which read as minutes would imply a whole HeLa run of eighty scans. MetaMorpheus .psmtsv and .osmtsv were already correct and are unaffected; of the three types implementing IQuantifiableResultFile, MSFragger was the only one wrong. 833 file-reading, quantifiable, FlashLFQ and adapter tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jp7uYMFdRCEWvg7sWZHFr2 * test(readers): pin the MSFragger retention-time divisor and drop change-narration Review follow-up to the seconds->minutes conversion. The gradient sanity check asserted only RetentionTime < 1.0 and RetentionTime == RetentionTimeInSeconds / 60.0; the first bound holds for any divisor >= 60 and the second restates the getter, so neither could catch a wrong conversion factor and an empty fixture would have passed vacuously. It now guards the record count (5) and asserts against fixture literals external to the property under test: the last scan equals 19.114 / 60 and the first-to-last span equals (19.114 - 1.9398) / 60, which fails if the divisor becomes 600 or 3600. Trim documentation that would rot. The two new <remarks> blocks on MsFraggerPsm narrated the bug history and baked in FlashLFQ internals — a consumer method name, one of its local variables, and the +/-2 minute window width — none of which this Readers record owns; they are reduced to the unit contract in the style the spectra readers already use, and the non-ASCII typography is gone. The change-narration comment in TestIdentificationAdapter describing what the assertion "previously read" is trimmed to the durable fact that the column is seconds. That history already lives in the commit and PR. Affected tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jp7uYMFdRCEWvg7sWZHFr2 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Broaden elastase|P from the six-residue set (A, V, S, G, L, I) to a fourteen-residue set, adding N, T, K, Y, Q, R, F and D. All motifs keep the existing X[P]| form, so the not-before-proline restriction is unchanged. The previous set matched the reagent vendor description of porcine pancreatic elastase. The expanded set reflects the much broader range of P1 residues observed empirically in elastase digests, where the enzyme behaves closer to semi-specific than its textbook A/V/S/G/L/I rule suggests. Update the elastase reverse-decoy expectation in PeptideWithSetModsDecoyTests accordingly. Decoy generation pins cleavage-site residues in place and reverses the remainder, so a larger motif set pins more positions: for KAYVPSRGHLDIN only V (blocked by the following P), P and H remain unpinned, giving KAYHPSRGVLDIN. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Nic Bollis <nbollis@comcast.net>
) Deepen the sequence-variant corpus along the substitution axis with 14 new oracle nodes (hand-derived from the bible, never captured from code): - S00-S02 sub before / on / after a PTM. S01 (T->S) pins the D7 case for a residue-specific motif -- dropped both positionally and motif-wise. - S03-S04 substitution at the protein start / end (in-range boundary). - S05-S06 VCF substitution passing / failing the minAlleleDepth cutoff. - S07-S09 bottom-up: a substitution moves the trypsin knife -- destroy (K->A), create (A->K), and block-before-proline (I->P under trypsin|P). First digestion-axis nodes (installment 5). - S10 multi-residue MNV (TI->VL) over a begin/end range, same length. - S11 variant-BORNE mod: V4->T carries a phospho stored on the variant (reciprocal of F03); round-trip gives it teeth on #1083 serialization. - SS00-SS01 double substitutions: independent, and one landing on the PTM. Harness: BuildMods now also registers variant-borne (VMOD) mods with the motif taken from the variant residue; BuildXml emits the nested <subfeature>/ <subposition> for VMOD and labels VCF subs as missense_variant. 62/62 green (31 nodes x Corpus_Node + Corpus_Node_RoundTrip). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Nic Bollis <nbollis@comcast.net>
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.
No description provided.