Implement start position read API (issue #23) #46
Conversation
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
- Document problem statement and desired behavior - Analyze current architecture and data flow - Propose API design options with recommendation - Detail implementation plan for block-level skipping - Include testing strategy and future considerations
Add read_channel_from method to TdmsFile that allows reading from a specific sample position, with block-level skipping optimization. Changes: - Add read_channel_from() method to TdmsFile with start position parameter - Refactor read_channel() to use read_channel_from() internally - Add read_single_from() and read_from() methods to DataBlock - Extend MultiChannelContigousReader with read_from() for offset support - Extend MultiChannelInterleavedReader with read_from() for offset support - Add comprehensive unit tests for offset reading functionality Performance optimization: - Skip entire data blocks when start position is beyond them - For contiguous layout: seek directly to offset within channel data - For interleaved layout: skip entire rows before reading The implementation maintains backward compatibility by keeping the original read_channel() method unchanged in its public API.
- Replace TdmsFile::open_file() with TdmsFile::load() in tests - Add missing std::path::Path import - Remove unnecessary 'mut' from channels parameter in read() methods
- Fix import path: use tedium::ChannelPath instead of tedium::paths::ChannelPath - Fix type mismatches in test: cast u64 to usize for array indexing - Move plans folder to docs/design for better organization
- Replace 'Group/Channel1' with 'structure/ch1' (actual test file channels) - Use common::open_test_file() helper instead of direct file loading - Remove unused imports (std::path::Path, TdmsFile)
The previous implementation incorrectly skipped entire sub-blocks (which contain data for ALL channels). For contiguous layout, we need to skip samples within each channel's contiguous data section. Changes: - Add read_sub_block_with_offset() method that skips samples within each channel - Skip by seeking forward by (samples_to_skip * element_size) bytes - Only read the remaining samples after the offset
- Fix trailing whitespace in contiguous_multi_channel_read.rs - Format long lines in test assertions to meet line length requirements - Split method chains across multiple lines for readability
kiloconnect
bot
force-pushed
the
session/agent_b39d2032-2b53-4f17-8481-02ba1015e5a9
branch
from
adc244b to
db31ca1
Compare
Implements uniform start position API for reading multiple channels with a shared start offset. This is efficient for time-aligned data where all channels share the same time base. Key changes: - Add read_channels_from() method to TdmsFile - Refactor read_channels() to use read_channels_from() internally - Make DataBlock::read_from() public for multi-channel offset reads - Extend ChannelProgress with offset tracking (samples_skipped, start_offset) - Add helper function get_block_samples() for block sample counting - Add comprehensive tests for multi-channel offset reading The implementation optimizes reading by: - Skipping entire data blocks when all channels have start > block samples - Using block-level offset for partial block reads - Tracking skip progress separately from read progress
Revises the multi-channel offset API to correctly handle channels written in separate blocks by tracking skip progress independently per channel. Key changes: - Simplify ChannelProgress to track samples_to_skip directly - Calculate per-channel skip amounts based on each channel's data in the block - Add read_with_per_channel_skip() to DataBlock for per-channel offsets - Implement read_with_per_channel_skip() in contiguous reader (independent seeks) - Implement read_with_per_channel_skip() in interleaved reader (min skip + discard) - Use fast path (existing read()) when no skip needed (90% case) - Use slow path (read_with_per_channel_skip()) only when needed This correctly handles the case where: Block 0: [Ch1: 1000 samples] Block 1: [Ch2: 1000 samples] read_channels_from([Ch1, Ch2], start=500) -> Ch1 skips 500 in Block 0, Ch2 skips 500 in Block 1
JamesWiresmith
requested changes
Jan 22, 2026
src/raw_data/mod.rs
Outdated
| pub fn read_with_per_channel_skip<'b, D: TdmsStorageType>( | ||
| &self, | ||
| reader: &mut (impl Read + Seek), | ||
| channels_to_read: &'b mut [(usize, &'b mut [D])], |
Contributor
There was a problem hiding this comment.
could skip be included in the channels to read structure - makes sense to comine it into one.
Addresses PR review feedback by bundling skip amounts with channel data instead of passing as separate parameter. This makes the API cleaner. Changes: - Update DataBlock::read_with_per_channel_skip signature to accept (index, buffer, skip) tuples - Add get_block_read_data_with_skip() helper function - Extract skip amounts from tuples in DataBlock implementation - Add test for channels written in separate blocks scenario - Add comprehensive design document combining all planning docs
Extract skip amounts before creating mutable references to avoid conflicting borrows.
The skip_amounts parameter only contains skips for channels present in the block, so we need to index into it sequentially rather than zipping with all channels.
- Format test arrays on multiple lines - Fix clippy::collapsible_if warning in interleaved reader
The skip should only be applied once at the beginning, not to each sub-block. Calculate how many complete sub-blocks to skip and apply remainder skip only to the first sub-block that's read. For uniform skip (read_from): - Skip complete sub-blocks by seeking - Apply remainder skip to first read sub-block - No skip for subsequent sub-blocks For per-channel skip (read_with_per_channel_skip): - Calculate per-channel sub-blocks to skip and remainders - Skip sub-blocks where all channels need to skip - Apply per-channel remainder skips to first read sub-block per channel - No skip for subsequent sub-blocks
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.
Summary
This PR implements the start position read API as outlined in the implementation plan (issue #23). It adds the ability to read channel data starting from a specific sample position, with optimizations for skipping entire data blocks.
Changes
New API
read_channel_from(channel, start, output)method toTdmsFileread_channel()to useread_channel_from()internally for consistencyImplementation Details
read_single_from()andread_from()methods toDataBlockMultiChannelContigousReaderwithread_from()for offset supportMultiChannelInterleavedReaderwithread_from()for offset supportPerformance Optimizations
Testing
tests/read_with_offset.rsBackward Compatibility
The implementation maintains full backward compatibility:
read_channel()method signature is unchangedread_channel_from()methodRelated Issues
Closes #23
Testing
All existing tests should pass, and new tests have been added to verify the offset reading functionality.
Documentation
The new method includes comprehensive documentation explaining: