Added update_parameters to EMA to fix calculation#4406
Merged
prabhat00155 merged 1 commit intopytorch:mainfrom Sep 14, 2021
Merged
Added update_parameters to EMA to fix calculation#4406prabhat00155 merged 1 commit intopytorch:mainfrom
prabhat00155 merged 1 commit intopytorch:mainfrom
Conversation
datumbox
approved these changes
Sep 14, 2021
Contributor
datumbox
left a comment
There was a problem hiding this comment.
LGTM, thanks Prabhat.
As @kazhang this is an issue on PyTorch core. Ideally the base class should include all state not just the params. We use a similar approach in other averaging schemes (see this) so this is aligned to what we've seen in the past. Though this workaround will do for our use-case, I think it's still worth raising it on PyTorch core and either correcting it or introducing better control over which params we should consider.
Contributor
Author
Makes sense. Will follow-up on this. |
16 tasks
facebook-github-bot
pushed a commit
to pytorch/pytorch
that referenced
this pull request
Sep 28, 2021
…65495) Summary: While implementing [EMA](pytorch/vision#4381 extends AveragedModel) in torchvision, update_parameters() from AveragedModel could not be used as it did not handle state_dict(), so a custom update_parameters() needed to be defined in [EMA class](pytorch/vision#4406). This PR aims to handle this scenario removing the need for this custom update_parameters() implementation. Discussion: pytorch/vision#4406 (review) Pull Request resolved: #65495 Reviewed By: datumbox Differential Revision: D31176742 Pulled By: prabhat00155 fbshipit-source-id: 326d14876018f21cf602bab5eaba344678dbabe2
facebook-github-bot
pushed a commit
that referenced
this pull request
Sep 30, 2021
Reviewed By: datumbox Differential Revision: D31268055 fbshipit-source-id: 2bedf7cd5db0a345dffa42a9ff94ce7d425e1008
prabhat00155
added a commit
to prabhat00155/pytorch
that referenced
this pull request
Oct 5, 2021
…ytorch#65495) Summary: While implementing [EMA](pytorch/vision#4381 extends AveragedModel) in torchvision, update_parameters() from AveragedModel could not be used as it did not handle state_dict(), so a custom update_parameters() needed to be defined in [EMA class](pytorch/vision#4406). This PR aims to handle this scenario removing the need for this custom update_parameters() implementation. Discussion: pytorch/vision#4406 (review) Pull Request resolved: pytorch#65495 Reviewed By: datumbox Differential Revision: D31176742 Pulled By: prabhat00155 fbshipit-source-id: 326d14876018f21cf602bab5eaba344678dbabe2 (cherry picked from commit 2ea724b)
malfet
pushed a commit
to pytorch/pytorch
that referenced
this pull request
Oct 6, 2021
…65495) (#65755) * Added option to update parameters using state_dict in AveragedModel (#65495) Summary: While implementing [EMA](pytorch/vision#4381 extends AveragedModel) in torchvision, update_parameters() from AveragedModel could not be used as it did not handle state_dict(), so a custom update_parameters() needed to be defined in [EMA class](pytorch/vision#4406). This PR aims to handle this scenario removing the need for this custom update_parameters() implementation. Discussion: pytorch/vision#4406 (review) Pull Request resolved: #65495 Reviewed By: datumbox Differential Revision: D31176742 Pulled By: prabhat00155 fbshipit-source-id: 326d14876018f21cf602bab5eaba344678dbabe2 (cherry picked from commit 2ea724b) * Added validation of mode parameter in AveragedModel (#65921) Summary: Discussion: #65495 (comment) Pull Request resolved: #65921 Reviewed By: albanD Differential Revision: D31310105 Pulled By: prabhat00155 fbshipit-source-id: 417691832a7c793744830c11e0ce53e3972d21a3 (cherry picked from commit c7748fc)
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
3 participants
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.
Resolves #4391.
Output logs:
resnext101_32x8d_training_log_ema_fix.txt
resnet18_training_log_ema_fix.txt
cc @datumbox