Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting num_cycles for cosine_with_restarts lr scheduler #3606

Merged
merged 1 commit into from Jun 5, 2023

Conversation

0x1355
Copy link
Contributor

@0x1355 0x1355 commented May 30, 2023

Expose num_cycles kwarg of get_schedule() through args.lr_num_cycles. This is similar to the implementation in current controlnet training script and other training scripts.

Note: different from the current controlnet training script, I scale args.lr_num_cycles by gradient_accumulation_steps, which seems to give the correct behavior.

Expose num_cycles kwarg of get_schedule() through args.lr_num_cycles.
@0x1355
Copy link
Contributor Author

0x1355 commented May 30, 2023

In the chart below, I set --lr_num_cycles=2. Orange line is the implementation where args.lr_num_cycles is scaled by gradient_accumulation_steps. Blue line is the unscaled version currently implemented in controlnet and other training scripts.

image

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented May 30, 2023

The documentation is not available anymore as the PR was closed or merged.

Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Ok for me! @sayakpaul wdyt?

Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the catch.

Would you like to also open a PR for ControlNet?

Cc: @yiyixuxu @williamberman

@sayakpaul sayakpaul merged commit de45af4 into huggingface:main Jun 5, 2023
7 checks passed
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
…ngface#3606)

Expose num_cycles kwarg of get_schedule() through args.lr_num_cycles.
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.

None yet

4 participants