Skip to content

Fix TslibDataModule generating synthetic time indices instead of real ones - #2342

Open
ShreyanshGoyal wants to merge 1 commit into
sktime:mainfrom
ShreyanshGoyal:fix/tslib-datamodule-time-index
Open

Fix TslibDataModule generating synthetic time indices instead of real ones#2342
ShreyanshGoyal wants to merge 1 commit into
sktime:mainfrom
ShreyanshGoyal:fix/tslib-datamodule-time-index

Conversation

@ShreyanshGoyal

Copy link
Copy Markdown

Summary

Fixes #2263.

_TslibDataset.__getitem__ built history_time_idx/future_time_idx from torch.arange(context_length) and torch.arange(context_length, context_length + prediction_length) — synthetic, always-zero-start positional indices — instead of the real per-series timestep values already computed in processed_data["timestep"]. The line reading from it was present in the source but commented out:

# history_time_idx = processed_data["timestep"][history_indices]
# future_time_idx = processed_data["timestep"][future_indices]
...
"history_time_idx": torch.arange(context_length),
"future_time_idx": torch.arange(context_length, context_length + prediction_length),

For any window that doesn't start at series position 0, or for irregular/gapped time axes, this silently fed models the wrong temporal context. Using the issue's repro (time_idx = [10, 20, 35, 50, 80, 120], context_length=3, prediction_length=2):

print(batch["history_time_idx"][0])  # tensor([0, 1, 2])  -- should be [10, 20, 35]
print(batch["future_time_idx"][0])   # tensor([3, 4])     -- should be [50, 80]

Fix

Restored history_time_idx/future_time_idx to read from processed_data["timestep"]. One wrinkle: TimeSeries.__getitem__ returns t as a plain numpy.ndarray (not a tensor), and a time column can legitimately be datetime64 (see test_multivariate_target in the existing test file, which uses pd.date_range) — torch.as_tensor rejects datetime64 arrays outright. Added a small _time_values_to_tensor helper that views datetime64 values as their underlying int64 ticks before conversion; plain int/float time columns pass through unchanged.

Test plan

  • Added test_time_idx_reflects_real_timestamps, using the exact irregular, non-zero-start reproduction from the issue — asserts history_time_idx/future_time_idx are a real, contiguous, in-order slice of the dataset's own time axis for every window. Confirmed this fails with the original synthetic-arange output on the pre-fix code and passes after the fix.
  • Added test_time_idx_supports_datetime_time_column, covering a datetime64 time column end-to-end (this would TypeError on a naive torch.as_tensor(...) fix without the datetime handling).
  • pytest pytorch_forecasting/data/tests/test_tslib_data_module.py -v — 15 passed, 1 failed. The failure (test_multivariate_target, AttributeError: 'list' object has no attribute 'shape') is the same pre-existing, unrelated failure flagged in the issue itself — confirmed by reproducing the identical failure against the unmodified file on main.
  • ruff check / ruff format --check on both changed files — clean (left one unrelated pre-existing formatting drift elsewhere in the source file untouched, to keep the diff focused).

… ones

_TslibDataset.__getitem__ built history_time_idx/future_time_idx from
torch.arange(context_length) and torch.arange(context_length,
context_length + prediction_length) -- synthetic, always-zero-start
positional indices -- instead of the real per-series timestep values already
computed in processed_data["timestep"] (the line reading from it was present
but commented out). For any window that doesn't start at series position 0,
or for irregular/gapped time axes, this silently fed models the wrong
temporal context: e.g. history [10, 20, 35] would report as [0, 1, 2].

Fixes sktime#2263.

The real timestep values are numpy arrays (numpy.ndarray from TimeSeries,
not necessarily plain integers -- datetime64 is a supported time column
type), and torch.as_tensor rejects datetime64 arrays outright, so a small
_time_values_to_tensor helper converts datetime64 to its underlying int64
ticks first.

Added test_time_idx_reflects_real_timestamps, using the exact irregular,
non-zero-start reproduction from the issue, asserting history/future time
values are a real, contiguous, in-order slice of the dataset's own time
axis (fails with the original synthetic-arange bug), and
test_time_idx_supports_datetime_time_column, covering a datetime64 time
column end-to-end.

Note: test_multivariate_target in the same file already fails on an
unrelated, pre-existing bug (list vs. tensor shape assertion) independent of
this change, as flagged in the issue -- confirmed by reproducing the same
failure on the unmodified file.
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 16.66667% with 25 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@c4f5aac). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...h_forecasting/data/tests/test_tslib_data_module.py 0.00% 24 Missing ⚠️
...forecasting/data/data_module/_tslib_data_module.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2342   +/-   ##
=======================================
  Coverage        ?   86.92%           
=======================================
  Files           ?      167           
  Lines           ?     9783           
  Branches        ?        0           
=======================================
  Hits            ?     8504           
  Misses          ?     1279           
  Partials        ?        0           
Flag Coverage Δ
cpu 86.92% <16.66%> (?)
pytest 86.92% <16.66%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] : Incorrect time index propagation in v2 datamodule output

1 participant