-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
extract manual optimization loop #9266
Conversation
for more information, see https://pre-commit.ci
b875f07
to
4cdfa9a
Compare
if result: | ||
# if no result, user decided to skip optimization | ||
# otherwise update running loss + reset accumulated loss | ||
self._update_running_loss(result.loss) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed this while resolving conflicts: this is not called anymore, is it? Is that intentional?
(personally lean towards removing the running loss, so this would be okay): see #9372
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically this should not have been removed, ... sigh
but yeah, we can see what happens with #9372
What does this PR do?
Part of #7938
Follow up to #9191
Fixes #9283
Now that automatic optimization is separated into its own loop, we can finally drastically simplify the calls to manual optimization and move it to a separat loop class.
TODO:
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
I made sure I had fun coding 🙃