Skip to content

fix(model): group DailyBatchSampler batches by datetime positions - #2323

Open
张弘 (shuxue6662-a11y) wants to merge 1 commit into
microsoft:mainfrom
shuxue6662-a11y:codex/fix-daily-batch-sampler
Open

fix(model): group DailyBatchSampler batches by datetime positions#2323
张弘 (shuxue6662-a11y) wants to merge 1 commit into
microsoft:mainfrom
shuxue6662-a11y:codex/fix-daily-batch-sampler

Conversation

@shuxue6662-a11y

Copy link
Copy Markdown

Description

DailyBatchSampler assumed that the rows of a trading day are contiguous in the data source: it computed per-day counts via groupby("datetime").size(), converted them to start offsets with cumsum, and sliced contiguous ranges np.arange(idx, idx + count).

TSDataSampler.get_index() swaps the level labels back to (datetime, instrument) but does NOT reorder the rows, which remain instrument-major (see "Indices design" in TSDataSampler). As a result each "daily" batch actually contained one instrument across many consecutive days instead of one day's cross-section.

This PR changes DailyBatchSampler to group the actual row positions by the datetime level, so every batch contains exactly one trading day's cross-section. It also:

  • fixes GATs.predict() to align predictions with the sampler's iteration order (dl_test.get_index()[np.concatenate(sampler_test.batches)]), since the previous dl_test.get_index() alignment assumed the old row order;
  • fixes __len__ to return the number of batches instead of the number of samples;
  • clarifies the TSDataSampler.get_index() docstring about the instrument-major row order;
  • adds regression tests covering the one-day-per-batch invariant, full sample coverage, and batch count.

Motivation and Context

Fixes #2319

How Has This Been Tested?

  • New regression tests: python -m pytest tests/test_daily_batch_sampler.py -> 2 passed
  • black --check -l 120 on the changed files
  • flake8 on the changed files
  • Full pipeline test (pytest qlib/tests/test_all_pipeline.py) -- requires downloading test data; covered by CI

Types of changes

  • Fix bugs
  • Add new feature
  • Update documentation

@shuxue6662-a11y

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

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.

DailyBatchSampler in pytorch_gats_ts.py yields cross-day batches: TSDataSampler.get_index() swaps index labels but not row order

1 participant