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

NLPDDPPlugin find_unused_parameters is configurable #3478

Merged

Conversation

mlgill
Copy link
Contributor

@mlgill mlgill commented Jan 20, 2022

This PR makes find_unused_parameters user configurable in NLPDDPPlugin by setting a default dictionary with the current value (False) and updating that dictionary with user provided kwargs. This ensures that the value is always False, even if it changes from its current default (also False) in PyTorch's DistributedDataParallel.

An alternative option would be to remove it from the initialization call to DistributedDataParallel since it is already False by default in PyTorch.

The reason for this request is that find_unused_parameters must be True for MegaMolBART.

I have read the contribution guidelines. All tests that are currently passing in main pass with this commit. (Note that two tests fail in NeMo main.) The code has been formatted with black and the commit is signed. This change was discussed with @ericharper several months ago.

Signed-off-by: Michelle Gill [email protected]

Copy link
Collaborator

@ericharper ericharper left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@ericharper ericharper merged commit fcdaaa1 into NVIDIA:main Feb 7, 2022
fayejf pushed a commit that referenced this pull request Mar 2, 2022
* NLPDDPPlugin find_unused_parameters is configurable

Signed-off-by: Michelle Gill <[email protected]>

* Choose simple example

* Update nlp_overrides.py

Co-authored-by: Eric Harper <[email protected]>
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