Experimental/recalib iter #46

jspaezp · 2025-10-16T17:06:49Z

Well this is not really meant to be a "real" PR (but it can become one).

I guess I added a lot of stuff that worked for me and wanted to know if any of it would be of interest.

Temperature aware calibration... enough said ... seems to work on my data ... I have no idea if it will work in other data types.
- Also modified the test files to include data for it.
Buffered read.
- I wanted to reduce the number of possible calls to the global allocator, so adding the option to pass a buffer means that I can read multiple frames with the same memory (instead of creating + destroyig vecs left and right)
- Speed difference is not huge in my tests...
TimsTofFile/Path api -> exposed it and allowed getting the metadata + frame reader from it (its nice to validate once that it is a real tims tof path and then dispatch instead of one validation per data type needed)
Frame struct: Split the data into a FrameMeta and a FramePeaks (frame composes from them), and the framereader reads the meta for each (instead of having a partially built Frame with empty vecs)

jspaezp · 2025-10-16T18:28:15Z

src/ms_data/frames.rs

+    /// have index 2 but its empty!
+    ///
+    /// Then this index can be converted using the Scan2ImConverter.convert
+    fn expand_mobility_iter(&self) -> impl Iterator<Item = u16> + '_ {


I think than an option to expand the peaks should be inside timsrust or at least this function

I have this set up internally with a more generic with a SparseVec structure though.

sounds good to me.

sander-willems-bruker · 2025-11-03T13:21:41Z

src/domain_converters.rs

+    // These two are here so we can implement optimized versions for iterators if needed.
+    fn convert_iter(
+        &self,
+        values: impl Iterator<Item = f64>,


I'd probably go for &[f64] rather than an impl

the reason I made it this way was for things like the scan index, where we dont have a materialized slice.

sander-willems-bruker · 2025-11-03T13:35:58Z

src/io/readers/file_readers/tdf_blob_reader/tdf_blobs.rs

+    pub(crate) fn decompress_reset(
+        &mut self,
+        compressed_bytes: &[u8],
+    ) -> Result<(), TdfBlobError> {
+        self.bytes.clear();
+        copy_decode(compressed_bytes, &mut self.bytes)
+            .map_err(|_| TdfBlobError::Decompression)?;
+        Self::check_len(self.bytes.as_slice())?;
+        Ok(())
+    }


Do you have some stats on how much better recycling allocated spae works compared to making new?

real answer ... no ... if my memory serves me correctly it was less than 5% in my laptop. But I believe that would greatly depend on what allocator is being used (I have seen massive heap allocation overhead in windows but not mac, and using mimalloc in windows makes it go away, whilst it makes mac slower ... )

sander-willems-bruker · 2025-11-03T13:41:51Z

src/ms_data/frames.rs

+    pub peaks: FramePeaks,
+    pub meta: FrameMeta,


This is a very good idea that I also have been wanting to implement for awhile already

sander-willems-bruker · 2025-11-03T13:42:28Z

src/ms_data/frames.rs

+    /// have index 2 but its empty!
+    ///
+    /// Then this index can be converted using the Scan2ImConverter.convert
+    fn expand_mobility_iter(&self) -> impl Iterator<Item = u16> + '_ {


sander-willems-bruker · 2025-11-03T13:43:08Z

src/ms_data/frames.rs

+    /// have index 2 but its empty!
+    ///
+    /// Then this index can be converted using the Scan2ImConverter.convert
+    fn expand_mobility_iter(&self) -> impl Iterator<Item = u16> + '_ {


I have this set up internally with a more generic with a SparseVec structure though.

sander-willems-bruker · 2025-11-03T13:46:47Z

src/ms_data/metadata.rs

I probably want to put the actual reading elsewhere. I understand the tight coupling between the reader and metadata, but might opt to move the metadata definition to the reader rather than the otherway around...

No preference... the main thing here was that the reader struct is never constructed, so it was kind of annoying to 'import' two structs for no reason.

sander-willems-bruker · 2025-11-03T13:52:31Z

src/io/readers/frame_reader.rs

+    pub dia_windows: Option<Vec<Arc<QuadrupoleSettings>>>,
+    pub frame_metas: Vec<FrameMeta>,
+    pub acquisition: AcquisitionType,
+    pub compression_type: u8,


In general, I feel structs shoud not have pub fields. I actually need to revisit all those that still have this... (in particular frame)

ok I think ... we can break this up into two things...

Should I have access to the data? -> I believe the answer is yes, I do not believe I should need to read the data to know how many MS1 frames I have, or how many quad types I am using. If you do not want to have that be pub that is cool, but a getter would be great to have.

Should I have "write-access" to the field. -> Well ... I think that if I am writing to it its one me to not mess it up.

sander-willems-bruker · 2025-11-03T13:54:34Z

src/io/readers/frame_reader.rs

+    /// Attempts to find the frame using the instrument index and
+    /// returns it if found.


Good to make explicit that the indices indeed are different in terms of accession (0-offset rather than 1)

I was not sure if that was the difference and if there could be gaps (which is why I used bin search instead of subtraction :P)

sander-willems-bruker · 2025-11-03T13:59:14Z

src/io/readers/frame_reader.rs

+        frame_buffer: &mut Frame,
+        blob_buffer: &mut TdfBlob,
+    ) -> Result<(), FrameReaderError> {
+        frame_buffer.peaks.clear();


At a very quick glance I find it difficult to see if/how this maintains multithreaded compatability without getting overly complex. Care to elaborate? If not, I'll try to look up the details later

the real answer is you dont :P if the user (me) wants to use this interface its on them to figure out the multi-threaded mutable state. (I use a per-thread buffer, but I can see someone else using an Arc<Mutex<Queue>>). You gain in alloc overhead but loose in threaded logic complexity as a user.

jspaezp added 3 commits September 16, 2025 16:14

experimental: initial branch commit

db7fe3f

chore: clippy+fmt

a05af52

tweak: simplified ims calculation

5f50da4

jspaezp commented Oct 16, 2025

View reviewed changes

sander-willems-bruker reviewed Nov 6, 2025

View reviewed changes

		/// Attempts to find the frame using the instrument index and
		/// returns it if found.

Experimental/recalib iter #46

Are you sure you want to change the base?

Experimental/recalib iter #46

Uh oh!

Conversation

jspaezp commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jspaezp commented Oct 16, 2025 •

edited

Loading