Fix daily batching for time-series samplers - #2322
Open
Serhii Ovsiienko (sergio12S) wants to merge 1 commit into
Open
Fix daily batching for time-series samplers#2322Serhii Ovsiienko (sergio12S) wants to merge 1 commit into
Serhii Ovsiienko (sergio12S) wants to merge 1 commit into
Conversation
Parafee41 (koriyoshi2041)
approved these changes
Aug 18, 2026
Parafee41 (koriyoshi2041)
left a comment
There was a problem hiding this comment.
Verified at 1d58db7. Grouping source-row positions by datetime fixes the instrument-major layout bug, and using the same sampler order for the prediction index keeps values aligned. The regression covers daily membership, batch count, and index ordering; the focused test passes locally, and the diff check is clean.
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.
TSDataSampler.get_index()exposes datetime/instrument labels over aninstrument-major row layout.
DailyBatchSamplerpreviously calculated correctper-date counts but converted them to contiguous ranges, so a nominal daily
batch could mix multiple dates.
I reproduced this with five dates and three instruments: every emitted batch
contained three distinct dates. This change groups the actual source-row
positions by datetime. Because that changes iteration order,
predict()nowbuilds its result index from the sampler's order as well, preventing otherwise
correct predictions from being assigned to the wrong observations.
__len__now returns the number of daily batches.
The added regression test constructs a real
TSDataSamplerand checks batchmembership, sampler length, and output-index alignment.
Tested with:
python -m pytest -q tests/model/test_pytorch_gats_ts.py— 1 passed.Closes #2319.