Implement Reader::iter_chromatograms (TIC chromatogram)#22
Open
Nabejo wants to merge 1 commit into
Open
Conversation
Wire the already-decoded Idx TIC data into a total ion current chromatogram (MS:1000235) via a SpectrumSource::iter_chromatograms override. One point per IdxRecord: time_sec from retention_time_min * 60.0, intensity from the record's tic (cps). No new raw-format decode work, only wiring existing fields into openmassspec_core::ChromatogramRecord, so TIC chromatograms now appear in the mzML <chromatogramList> OpenSXRaw produces. The per-spectrum SpectrumRecord.total_ion_current field is left None as before: that value must match sum(raw intensities) for the conformance suite's rel_close check, which is a per-spectrum constraint and does not apply to a separate chromatogram trace (assert_source_invariants never inspects iter_chromatograms). BPC and SRM/MRM are intentionally out of scope: a per-scan base peak is not decoded today, and no Q1/Q3 transition-level data is decoded anywhere in this reader. Both need net-new decode work and should be separate follow-up issues. Adds unit tests for the chromatogram wiring and a corpus-gated integration test. Refs Sigilweaver#21 Claude-Session: https://claude.ai/code/session_01TLSifJpmaN7RvW3xi2uWgo
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.
Fixes #21
Summary
Implements
Reader::iter_chromatogramsas aSpectrumSourceoverride incrates/opensxraw/src/reader.rs, emitting exactly one total ion current chromatogram (MS:1000235"total ion current chromatogram") built entirely from the already-decodedidx_records:IdxRecordtime_sec=retention_time_min * 60.0intensity= the record's Idxtic(cps)precursor_mz/product_mz=NoneThis is a pure wire-up of existing decoded fields into
openmassspec_core::ChromatogramRecord; no new raw-format decode work is involved. As a result, TIC chromatograms now appear in the<chromatogramList>of the mzML OpenSXRaw produces, instead of falling through to the empty default impl.Why the per-spectrum TIC reasoning does not apply here
iter_spectradeliberately leavesSpectrumRecord.total_ion_currentasNonebecause the Idx TIC is in cps (physically calibrated) and would fail the conformance suite'srel_closecheck againstsum(raw intensities). That constraint is specific to the per-spectrum field. A TIC chromatogram is a separate trace of (retention time, cps) points;assert_source_invariantsnever inspectsiter_chromatograms, so the mismatch does not apply. The per-spectrum field is leftNoneexactly as before, so existing conformance behaviour is unchanged.Scope: TIC only
BPC and SRM/MRM are intentionally left out, as the issue notes they need materially different net-new decode work:
base_peak_mz/base_peak_intensityare hardcodedNone).Suggest tracking both as separate follow-up issues if pursued.
Testing
cargo fmt --all -- --check- cleancargo clippy --all-targets -- -D warnings- cleancargo test --workspace- all passAdded:
reader.rs(syntheticidx_records, no corpus needed): verify a single TIC record with the correct CV term,Noneprecursor/product, and thattime_sec/intensityderive fromretention_time_min/tic(including a large-cps regression guard confirming the value passes through verbatim rather than being tied to the per-spectrum field).tests/conformance.rs(skips cleanly when the out-of-tree corpus is absent) asserting exactly one TIC chromatogram with one point per Idx record.The existing conformance suite continues to pass unchanged.
Clean-room note
TIC values come only from bytes already parsed from the
.wiffcontainer by this project's own code. No comparison against Analyst / SCIEX OS / MultiQuant / msconvert output was made or used in verifying this feature, per CONTRIBUTING.md.(Contributed by @Nabejo.)