Skip to content

Commit e7a9155

Browse files
committed
fix(opentimstdf): stream iter_spectra frame-by-frame instead of buffering the run
OwnedTdfSource/TdfSource::iter_spectra eagerly decoded and memoized every frame into a Vec on first call, then cloned it for the returned iterator - two full copies of the run's spectra live at once, on top of never being lazy in the first place. Replaced with a std::iter::from_fn generator that decodes one frame at a time, queuing only that frame's PASEF/diaPASEF fan-out before moving on. A frame that fails to decode is now skipped rather than aborting the whole run, matching SpectrumSource::iter_spectra's documented silent-skip contract. Closes #4.
1 parent af14ebd commit e7a9155

1 file changed

Lines changed: 90 additions & 92 deletions

File tree

crates/opentimstdf/src/mzml.rs

Lines changed: 90 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@
2626
//! unique).
2727
//!
2828
//! Frames with `msms_type` other than 0/8/9 (e.g. PRM-PASEF = 10) are
29-
//! skipped for now. Decode errors on individual frames bubble up to abort
30-
//! the iteration; the canonical writer trusts whatever the iterator
31-
//! yields.
29+
//! skipped for now. `iter_spectra` decodes one frame at a time as the
30+
//! iterator is driven; a frame that fails to decode is skipped rather than
31+
//! aborting the whole run, per [`msc::SpectrumSource::iter_spectra`]'s
32+
//! "skip silently" contract - the canonical writer trusts whatever the
33+
//! iterator yields.
3234
3335
use std::io::Write;
3436
use std::path::Path;
@@ -333,18 +335,17 @@ fn build_dia_ms2(
333335
/// Construct with [`TdfSource::new`]; iterate via the trait's
334336
/// [`iter_spectra`](openmassspec_core::SpectrumSource::iter_spectra) method.
335337
///
336-
/// The iterator buffers every spectrum in memory up front. timsTOF data is
337-
/// typically O(10 GB) raw / O(GB) decoded; for very large runs the caller
338-
/// should iterate frame-by-frame using the lower-level [`Reader`] API and
339-
/// build their own [`openmassspec_core::SpectrumSource`].
338+
/// `iter_spectra` decodes and projects one frame at a time as the returned
339+
/// iterator is driven, so memory use is bounded by a single frame's worth
340+
/// of spectra (PASEF/diaPASEF frames fan out into several spectra each,
341+
/// which are queued and drained before the next frame is decoded) rather
342+
/// than the whole run.
340343
pub struct TdfSource<'a> {
341344
reader: &'a Reader,
342345
bundle_name: String,
343346
metadata: Metadata,
344347
calibration: Calibration,
345348
frames: Vec<Frame>,
346-
/// Pre-built spectra; populated lazily on the first `iter_spectra` call.
347-
spectra: Option<Vec<msc::SpectrumRecord>>,
348349
}
349350

350351
impl<'a> TdfSource<'a> {
@@ -360,7 +361,6 @@ impl<'a> TdfSource<'a> {
360361
metadata,
361362
calibration,
362363
frames,
363-
spectra: None,
364364
})
365365
}
366366

@@ -383,21 +383,8 @@ impl<'a> TdfSource<'a> {
383383
metadata,
384384
calibration,
385385
frames,
386-
spectra: None,
387386
})
388387
}
389-
390-
fn build_spectra(&mut self) -> Result<&Vec<msc::SpectrumRecord>> {
391-
if self.spectra.is_none() {
392-
self.spectra = Some(project_frames(
393-
self.reader,
394-
&self.frames,
395-
&self.calibration,
396-
)?);
397-
}
398-
#[allow(clippy::expect_used)]
399-
Ok(self.spectra.as_ref().expect("populated above"))
400-
}
401388
}
402389

403390
/// Self-contained variant of [`TdfSource`] that owns its [`Reader`]. Used
@@ -408,69 +395,93 @@ pub struct OwnedTdfSource {
408395
metadata: Metadata,
409396
calibration: Calibration,
410397
frames: Vec<Frame>,
411-
spectra: Option<Vec<msc::SpectrumRecord>>,
412398
}
413399

414-
impl OwnedTdfSource {
415-
fn build_spectra(&mut self) -> Result<&Vec<msc::SpectrumRecord>> {
416-
if self.spectra.is_none() {
417-
self.spectra = Some(project_frames(
418-
&self.reader,
419-
&self.frames,
420-
&self.calibration,
421-
)?);
422-
}
423-
#[allow(clippy::expect_used)]
424-
Ok(self.spectra.as_ref().expect("populated above"))
425-
}
426-
}
427-
428-
fn project_frames(
400+
/// Project one frame into zero or more spectra, incrementing `scan_counter`
401+
/// for each spectrum produced. Any decode failure - the frame's peaks, its
402+
/// PASEF info rows, or its diaPASEF windows - causes that frame to be
403+
/// skipped (returns an empty `Vec`) rather than aborting the caller's
404+
/// iteration, matching [`msc::SpectrumSource::iter_spectra`]'s silent-skip
405+
/// contract. A precursor lookup failure for a single PASEF MS2 is narrower:
406+
/// it only drops that spectrum's precursor metadata, not the spectrum.
407+
fn spectra_for_frame(
429408
reader: &Reader,
430-
frames: &[Frame],
409+
frame: &Frame,
431410
calibration: &Calibration,
432-
) -> Result<Vec<msc::SpectrumRecord>> {
433-
let mut spectra: Vec<msc::SpectrumRecord> = Vec::with_capacity(frames.len());
434-
let mut scan_counter: u32 = 0;
435-
for frame in frames {
436-
let peaks = reader.decode_peaks(frame)?;
437-
match frame.msms_type {
438-
0 => {
439-
scan_counter += 1;
440-
spectra.push(build_ms1(scan_counter, frame, &peaks, calibration));
441-
}
442-
8 => {
443-
let infos = reader.pasef_msms_info_for_frame(frame.id)?;
444-
for info in infos {
445-
let prec = reader.precursor(info.precursor_id)?;
446-
scan_counter += 1;
447-
spectra.push(build_pasef_ms2(
448-
scan_counter,
449-
frame,
450-
&info,
451-
prec.as_ref(),
452-
&peaks,
453-
calibration,
454-
));
455-
}
411+
scan_counter: &mut u32,
412+
) -> Vec<msc::SpectrumRecord> {
413+
let Ok(peaks) = reader.decode_peaks(frame) else {
414+
return Vec::new();
415+
};
416+
match frame.msms_type {
417+
0 => {
418+
*scan_counter += 1;
419+
vec![build_ms1(*scan_counter, frame, &peaks, calibration)]
420+
}
421+
8 => {
422+
let Ok(infos) = reader.pasef_msms_info_for_frame(frame.id) else {
423+
return Vec::new();
424+
};
425+
let mut out = Vec::with_capacity(infos.len());
426+
for info in infos {
427+
let prec = reader.precursor(info.precursor_id).ok().flatten();
428+
*scan_counter += 1;
429+
out.push(build_pasef_ms2(
430+
*scan_counter,
431+
frame,
432+
&info,
433+
prec.as_ref(),
434+
&peaks,
435+
calibration,
436+
));
456437
}
457-
9 => {
458-
let windows = reader.dia_windows_for_frame(frame.id)?;
459-
if let Some(group) = windows {
460-
for w in &group.windows {
461-
scan_counter += 1;
462-
spectra.push(build_dia_ms2(scan_counter, frame, w, &peaks, calibration));
463-
}
438+
out
439+
}
440+
9 => {
441+
let Ok(windows) = reader.dia_windows_for_frame(frame.id) else {
442+
return Vec::new();
443+
};
444+
let mut out = Vec::new();
445+
if let Some(group) = windows {
446+
for w in &group.windows {
447+
*scan_counter += 1;
448+
out.push(build_dia_ms2(*scan_counter, frame, w, &peaks, calibration));
464449
}
465450
}
466-
_ => continue,
451+
out
467452
}
453+
_ => Vec::new(),
468454
}
469-
Ok(spectra)
470455
}
471456

472-
fn run_metadata_for(meta: &Metadata, bundle_name: &str, n_spectra: usize) -> msc::RunMetadata {
473-
let _ = n_spectra;
457+
/// Build a lazy, frame-at-a-time spectrum iterator. Decodes and projects
458+
/// one frame per call to `next()` that yields nothing from the pending
459+
/// queue, so at most one frame's peaks (plus its fan-out of PASEF/diaPASEF
460+
/// spectra) are held in memory at a time.
461+
fn frame_iter<'s>(
462+
reader: &'s Reader,
463+
frames: &'s [Frame],
464+
calibration: &'s Calibration,
465+
) -> impl Iterator<Item = msc::SpectrumRecord> + 's {
466+
let mut frame_idx = 0usize;
467+
let mut pending = std::collections::VecDeque::new();
468+
let mut scan_counter: u32 = 0;
469+
std::iter::from_fn(move || loop {
470+
if let Some(rec) = pending.pop_front() {
471+
return Some(rec);
472+
}
473+
let frame = frames.get(frame_idx)?;
474+
frame_idx += 1;
475+
pending.extend(spectra_for_frame(
476+
reader,
477+
frame,
478+
calibration,
479+
&mut scan_counter,
480+
));
481+
})
482+
}
483+
484+
fn run_metadata_for(meta: &Metadata, bundle_name: &str) -> msc::RunMetadata {
474485
msc::RunMetadata {
475486
source_file_name: bundle_name.to_string(),
476487
source_file_format: source_file_format_cv(),
@@ -485,29 +496,19 @@ fn run_metadata_for(meta: &Metadata, bundle_name: &str, n_spectra: usize) -> msc
485496

486497
impl<'a> msc::SpectrumSource for TdfSource<'a> {
487498
fn run_metadata(&self) -> msc::RunMetadata {
488-
let n = self.spectra.as_ref().map(|v| v.len()).unwrap_or(0);
489-
run_metadata_for(&self.metadata, &self.bundle_name, n)
499+
run_metadata_for(&self.metadata, &self.bundle_name)
490500
}
491501
fn iter_spectra<'s>(&'s mut self) -> Box<dyn Iterator<Item = msc::SpectrumRecord> + 's> {
492-
let recs = self.build_spectra().cloned().unwrap_or_default();
493-
Box::new(recs.into_iter())
494-
}
495-
fn spectrum_count_hint(&self) -> Option<usize> {
496-
self.spectra.as_ref().map(|v| v.len())
502+
Box::new(frame_iter(self.reader, &self.frames, &self.calibration))
497503
}
498504
}
499505

500506
impl msc::SpectrumSource for OwnedTdfSource {
501507
fn run_metadata(&self) -> msc::RunMetadata {
502-
let n = self.spectra.as_ref().map(|v| v.len()).unwrap_or(0);
503-
run_metadata_for(&self.metadata, &self.bundle_name, n)
508+
run_metadata_for(&self.metadata, &self.bundle_name)
504509
}
505510
fn iter_spectra<'s>(&'s mut self) -> Box<dyn Iterator<Item = msc::SpectrumRecord> + 's> {
506-
let recs = self.build_spectra().cloned().unwrap_or_default();
507-
Box::new(recs.into_iter())
508-
}
509-
fn spectrum_count_hint(&self) -> Option<usize> {
510-
self.spectra.as_ref().map(|v| v.len())
511+
Box::new(frame_iter(&self.reader, &self.frames, &self.calibration))
511512
}
512513
}
513514

@@ -518,16 +519,13 @@ impl msc::SpectrumSource for OwnedTdfSource {
518519
/// valid mzML 1.1.0 document via the canonical writer in `openmassspec-core`.
519520
pub fn write_mzml<P: AsRef<Path>, W: Write>(bundle_dir: P, out: &mut W) -> Result<()> {
520521
let mut src = TdfSource::open(bundle_dir)?;
521-
// Eagerly build so spectrum_count_hint is populated for the writer.
522-
src.build_spectra()?;
523522
msc::write_mzml(&mut src, out)?;
524523
Ok(())
525524
}
526525

527526
/// Indexed-mzML equivalent of [`write_mzml`].
528527
pub fn write_indexed_mzml<P: AsRef<Path>, W: Write>(bundle_dir: P, out: &mut W) -> Result<()> {
529528
let mut src = TdfSource::open(bundle_dir)?;
530-
src.build_spectra()?;
531529
msc::write_indexed_mzml(&mut src, out)?;
532530
Ok(())
533531
}

0 commit comments

Comments
 (0)