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

[Feature] Support dynamic EMA #261

Merged
merged 4 commits into from
Mar 8, 2022
Merged

Conversation

plyfager
Copy link
Collaborator

@plyfager plyfager commented Mar 7, 2022

This PR modify ema hook, refer to https://github.com/NVlabs/stylegan3.

  1. Support update momentum in EMA Hook.
  2. Add related unittest.

@plyfager plyfager changed the title support update ema momentum [WIP] Support dynamic EMA Mar 7, 2022
@plyfager plyfager changed the title [WIP] Support dynamic EMA [Feature] Support dynamic EMA Mar 8, 2022
momentum_cfg)
self.momentum_policy = momentum_policy
if momentum_policy != 'fixed':
assert hasattr(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest that we can add a class attribute called _supported_momentum_policy. Use instance attributes' names to check the policy may not be safe.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True, may be we can register interp_func and momentum_updater in this class?


@staticmethod
def lerp(a, b, momentum=0.999, momentum_nontrainable=0., trainable=True):
m = momentum if trainable else momentum_nontrainable
return a + (b - a) * m

@staticmethod
def rampup(runner, ema_kimg=10, ema_rampup=0.05, batch_size=4, eps=1e-8):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should change the naming template of momentum update policy as {POLICY}_momentum_updater or get_{POLICY}_momentum.
Because we had already used the name of interpolating policy (e.g. lerp) as the function name. Using momentum update policy as function name may lead to confusion.

Copy link
Collaborator Author

@plyfager plyfager Mar 8, 2022

Choose a reason for hiding this comment

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

may be we can register interp_func and momentum_updater in this class? Keep these functions in two separate function dictionaries.

@plyfager plyfager merged commit e607c0c into open-mmlab:master Mar 8, 2022
LeoXing1996 pushed a commit that referenced this pull request Jul 16, 2022
* support update ema momentum

* complete docstring

* fix lint

* fix as comment
LeoXing1996 pushed a commit to LeoXing1996/mmgeneration that referenced this pull request Oct 27, 2022
* tiny fix

* Tiny Fix, add limited_gpu.

* Tiny Fix

* Tiny Fix

Co-authored-by: liyinshuo <liyinshuo@sensetime.com>
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.

2 participants