Skip to content

Disable the profiler on an empty profiling window instead of raising - #2088

Open
gutianyu-google wants to merge 1 commit into
google:mainfrom
gutianyu-google:sft-profiler-fail-open
Open

Disable the profiler on an empty profiling window instead of raising#2088
gutianyu-google wants to merge 1 commit into
google:mainfrom
gutianyu-google:sft-profiler-fail-open

Conversation

@gutianyu-google

Copy link
Copy Markdown

Profiler.__init__ raised ValueError when the first profile step was not before the last one. That window is derived from the trainer's initial_step and max_step plus the profiler options, and in RL setups the PeftTrainer is constructed with initial_step == max_step, so passing any profiler_options at all aborted the run at trainer construction, well after the models had loaded onto the accelerators.

An empty profiling window is a misconfiguration of profiling, not of the training run. This PR warns with the derived step numbers and disables profiling for that instance, matching how the profiler already behaves when no options are given or on a non-primary process. Valid windows are unaffected.

The existing test_invalid_step_numbers cases are kept and now assert the warning, the disabled state, and that maybe_activate does not start a trace.

Checklist

  • I have added all the necessary unit tests for my change.
  • I have verified that my change does not break existing code and all unit tests pass.
  • I have added all appropriate doc-strings/documentation.
  • My PR is based on the latest changes of the main branch (if unsure, rebase the code).
  • I have signed the Contributor License Agreement.
  • I have followed Contribution Guidelines.

@google-cla

google-cla Bot commented Sep 2, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

`Profiler.__init__` raised ValueError when the first profile step was not
before the last one. That window is derived from the trainer's initial and
max steps plus the profiler options, and in RL setups the PeftTrainer is
constructed with initial_step == max_step, so any profiler_options at all
aborted the run at construction time, well after the models had loaded.

An empty profiling window is a misconfiguration of profiling, not of the
training run. Warn with the derived step numbers and disable profiling,
matching how the profiler already behaves without options or on a
non-primary process.
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