Skip to content

expose polynomial:power and cosine_with_restarts:num_cycles params #1737

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Dec 20, 2022
Merged

expose polynomial:power and cosine_with_restarts:num_cycles params #1737

merged 4 commits into from
Dec 20, 2022

Conversation

zetyquickly
Copy link
Contributor

to get_scheduler func,

  • add it to the train_dreambooth.py

@zetyquickly
expose polynomial:power and cosine_with_restarts:num_cycles using get…
f0c5a0b 
…_scheduler func, add it to train_dreambooth.py
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Dec 17, 2022 •
edited
Loading

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

@zetyquickly
Copy link
Contributor Author

@pcuenca PR to an issue #1232

What's done:

  • exposed only two parameters power and num_cycles on top of existing get_scheduler func API. it's possible to expose all of the parameters but taking into account that the issue wasn't there before we've noticed it, I assume it would pile up the code
  • changed parameter num_cycles name of the cosine scheduler to not overlap with cosine_with_restarts, because they have different meanings
  • added new parameters to the dreambooth's args

Haven't found any test script in test folder that tests optimizers.py

pcuenca
pcuenca approved these changes Dec 19, 2022
View reviewed changes
Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

Looks good!

An alternative would be to use scheduler_kwargs in get_scheduler, but since we only have those two configurable schedulers I think it's good for now.

@pcuenca pcuenca requested a review from patil-suraj December 19, 2022 10:11
@zetyquickly @pcuenca
Update src/diffusers/optimization.py
b7554ce 
Co-authored-by: Pedro Cuenca <pedro@huggingface.co>
patrickvonplaten
patrickvonplaten approved these changes Dec 20, 2022
View reviewed changes
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.

Thanks @zetyquickly

@patrickvonplaten patrickvonplaten merged commit d87cc15 into huggingface:main Dec 20, 2022
sliard pushed a commit to sliard/diffusers that referenced this pull request Dec 21, 2022
@zetyquickly @pcuenca @sliard
expose polynomial:power and cosine_with_restarts:num_cycles params (h…
5ac7f28 
…uggingface#1737)

* expose polynomial:power and cosine_with_restarts:num_cycles using get_scheduler func, add it to train_dreambooth.py

* fix formatting

* fix style

* Update src/diffusers/optimization.py

Co-authored-by: Pedro Cuenca <pedro@huggingface.co>

Co-authored-by: Pedro Cuenca <pedro@huggingface.co>
@zetyquickly zetyquickly mentioned this pull request Feb 25, 2023
Closed
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
@zetyquickly @pcuenca
expose polynomial:power and cosine_with_restarts:num_cycles params (h…
2831244 
…uggingface#1737)

* expose polynomial:power and cosine_with_restarts:num_cycles using get_scheduler func, add it to train_dreambooth.py

* fix formatting

* fix style

* Update src/diffusers/optimization.py

Co-authored-by: Pedro Cuenca <pedro@huggingface.co>

Co-authored-by: Pedro Cuenca <pedro@huggingface.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pcuenca pcuenca pcuenca approved these changes

@patil-suraj patil-suraj Awaiting requested review from patil-suraj

+1 more reviewer

@patrickvonplaten patrickvonplaten patrickvonplaten approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zetyquickly @HuggingFaceDocBuilderDev @pcuenca @patrickvonplaten