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

OptimWrapper sets same param groups as Optimizer #3821

Merged
merged 1 commit into from Oct 19, 2022

Conversation

warner-benjamin
Copy link
Collaborator

This PR harmonizes the default parameter group setting between OptimWrapper and Optimizer by modifying OptimWrapper to match Optimizer's logic.

Currently, when passed the default Learner.splitter of trainable_params, Optimizer creates one parameter group for the entire model but OptimWrapper creates a parameter group for every trainable parameter. For example, ResNet50 results with ~160 parameter groups instead of the expected one parameter group.

Users can provide their own parameter groups by setting convert_groups=False and passing in a dictionary to params as before.

This PR also modifies OptimWrapper.hypers to be compatible with a larger number of PyTorch optimizers by allowing new param group dictionary keys to be added during training. Nvidia's Apex optimizers are one example with this behavior with FusedAdam adding 'step' to each parameter group during training but not on optimizer creation.

This also makes OptimWrapper forward compatible with new param group additions not currently in pytorch_hp_map, such as 'fused' coming in a future PyTorch release.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@warner-benjamin
Copy link
Collaborator Author

I'm reasonably certain the CI error is not related to my PR and it needs to be reran later when the networking issues are resolved.

ERROR: Could not install packages due to an OSError: HTTPSConnectionPool(host='files.pythonhosted.org', port=443): Max retries exceeded with url:

@jph00
Copy link
Member

jph00 commented Oct 19, 2022

Many thanks @warner-benjamin !

@jph00 jph00 merged commit eddc084 into fastai:master Oct 19, 2022
@warner-benjamin warner-benjamin deleted the optimwrapper branch November 1, 2022 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants