Skip to content

Fix timer initialization in StepTimeCallback#946

Open
yuxin00j wants to merge 1 commit into
fsspec:mainfrom
yuxin00j:fix-step-time-callback
Open

Fix timer initialization in StepTimeCallback#946
yuxin00j wants to merge 1 commit into
fsspec:mainfrom
yuxin00j:fix-step-time-callback

Conversation

@yuxin00j

@yuxin00j yuxin00j commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Updates the StepTimeCallback within the PyTorch Lightning training script to correctly initialize timers at the very beginning of training rather than at the start of each epoch.

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.68%. Comparing base (25d99d9) to head (1f6a023).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #946   +/-   ##
=======================================
  Coverage   89.68%   89.68%           
=======================================
  Files          16       16           
  Lines        3579     3579           
=======================================
  Hits         3210     3210           
  Misses        369      369           

☔ 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.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request removes the on_train_epoch_start callback from the PyTorch Lightning training simulation script. The reviewer noted that removing this callback will introduce a bug in multi-epoch training, as the first step of subsequent epochs would incorrectly include inter-epoch overhead, thereby inflating the measured step time. It is recommended to retain both on_train_start and on_train_epoch_start to ensure accurate timing metrics.

@yuxin00j yuxin00j marked this pull request as ready for review July 3, 2026 06:25
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.

2 participants