[textual_inversion.py] Fix the LR scheduler when num_train_epochs is passed in a distributed training env - #14546
Open
adi-IL wants to merge 1 commit into
Open
Conversation
Scale warmup and training steps by accelerator.num_processes so --num_train_epochs builds the same schedule as the other official trainers after huggingface#8312. Tracks huggingface#8384.
Contributor
|
Hi @adi-IL, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8384
Fixes the LR scheduler step miscalculation in
examples/textual_inversion/textual_inversion.pywhen--num_train_epochsis used with distributed training.What was wrong
When
--max_train_stepsis None, the script computednum_update_steps_per_epochfrom the unsharded dataloader length and setmax_train_steps = num_train_epochs * num_update_steps_per_epochbeforeaccelerator.prepare. After sharding, the effective dataloader length isceil(len(dataloader)/num_processes), so the scheduler was created withtraining_steps = max_train_steps * num_processeswhile the actual optimization steps were smaller. The mismatch caused the scheduler to step incorrectly andmax_train_stepsto be recomputed without the sharding guard.What changed
Mirrors the fix already applied to
textual_inversion_sdxl.pyin #11557 and the pattern from #8312. Now computeslen_train_dataloader_after_sharding, derivesnum_update_steps_per_epochfrom the sharded length, and setsnum_training_steps_for_scheduler = num_train_epochs * num_update_steps_per_epoch * num_processes(ormax_train_steps * num_processeswhen provided). Introducesnum_warmup_steps_for_schedulerand defersmax_train_stepsassignment until afteraccelerator.preparewith the warning guard when lengths do not match. One file, one script as requested in #8384.Coordination
Tracks #8384. The issue lists this script as
[ ] textual_inversion.pyand says to target one script per PR and mention @sayakpaul and @geniuspatrick for review. Standing permission for this checklist item, same as the already merged siblings #14527, #14528, #14540.Test plan
/home/adi-IL/Desktop/.gh-pr-routine/.venv/bin/python -m py_compile examples/textual_inversion/textual_inversion.py-> py_compile OKpython -c "import ast; ast.parse(open('examples/textual_inversion/textual_inversion.py').read())"-> ast parse oknum_training_steps_for_schedulerequalsmax_train_steps * num_processesafter sharding, while the old code produced double or mismatched steps.Self-review notes
Checked against
.ai/references/review-rules.md: no new dependencies, no model or pipeline changes, preserves existing docstrings and comments, matches the sibling fix intextual_inversion_sdxl.py:802-853, no dead code, only the scheduler math block changed. The warning uses the same message as the sibling.cc @sayakpaul @geniuspatrick