Skip to content

Honor calibration batch ranges and reset the reader on rewind - #2689

Open
Sylvester Kaczmarek (sylvesterkaczmarek) wants to merge 1 commit into
microsoft:mainfrom
sylvesterkaczmarek:fix/calibration-reader-range-position
Open

Sylvester Kaczmarek (sylvesterkaczmarek) wants to merge 1 commit into
microsoft:mainfrom
sylvesterkaczmarek:fix/calibration-reader-range-position

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown
Contributor

Describe your changes

Fixes #2688.

The default calibration reader updates its range counters without positioning the underlying iterator. Requesting batches [2, 4) from a fresh reader returns batches 0 and 1. Rewinding a bounded reader can also return no data because the current index is not reset.

This change positions the iterator when the requested start differs from the current index, reuses the existing iterator for contiguous windows, and resets the current index on rewind while preserving the configured end bound.

Default unbounded iteration, input conversion, labels, and reader length are unchanged.

Tests

  • 20 new regression cases pass; 14 fail on unchanged upstream.
  • The new cases plus related data-container tests pass: 29 tests.
  • A real ONNX Runtime MinMaxCalibrater regression verifies that range [2, 4) produces extrema [2, 3], rather than the incorrect [0, 1].
  • Changed-file lintrunner -a passes.
  • Direct wheel build succeeds, and all 20 regression cases pass against the installed wheel outside the checkout.
  • git diff --check passes.

Checklist before requesting a review

  • Add unit tests for this change.
  • Targeted tests pass.
  • Lint and apply fixes with lintrunner -a.
  • No public API or configuration change requiring documentation updates.

Issue link

#2688

Copilot AI lite review requested due to automatic review settings September 25, 2026 11:44
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

Reviewed changes address the stated calibration range and rewind issues with regression coverage.

Review effort: Lite
Findings: None

What changed in this PR

Fixes calibration batch-range selection and reader rewind behavior.

Changes:

  • Repositions iterators for noncontiguous ranges.
  • Resets reader state on rewind while preserving bounds.
  • Adds regression and ONNX Runtime calibration tests.
File Description
test/​data_container/​test_calibration_dataloader_ranges.py Adds range, rewind, and calibration regression coverage.
olive/​data/​component/​dataloader.py Corrects range positioning and rewind behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

This branch has not been deployed

No deployments
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.

Calibration reader uses the wrong batches after set_range and cannot replay bounded ranges

2 participants