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

fix error reported 'find_unused_parameters' running in mutiple GPUs #5355

Merged
merged 3 commits into from
Oct 27, 2023

Conversation

jiaqiw09
Copy link
Contributor

What does this PR do?

Fixes #5354

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline?
  • Did you read our philosophy doc (important for complex PRs)?
  • Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings. not required
  • Did you write any new necessary tests? not required

Who can review?

@DN6 @patrickvonplaten

@jiaqiw09
Copy link
Contributor Author

jiaqiw09 commented Oct 10, 2023

@DN6 @patrickvonplaten

In this PR, I just change files related to UT cases I mentioned in issue.
If you think it is a suitable solution and it's necessary to add those codes to all train files in example folder, feel free to comment this PR.

Best wishes

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Oct 11, 2023

The documentation is not available anymore as the PR was closed or merged.

@jiaqiw09
Copy link
Contributor Author

@DN6 hi, I just check the 'Run code quality checks ', and the problem maybe the this line

from accelerate.utils import ProjectConfiguration, set_seed, DistributedDataParallelKwargs

if I use isort module to do some fix, it turns to

from accelerate.utils import (DistributedDataParallelKwargs,
                              ProjectConfiguration, set_seed)

However, I use isort to check file train_text_to_image_lora_sdxl.py and train_dreambooth_lora_sdxl.py, there are mutiple lines are fixed by isort module. As I can't do entire test locally, would you mind having a look?

  • If I only need change from accelerate.utils import ProjectConfiguration, set_seed, DistributedDataParallelKwargs, I can submit an commit quickly.
  • Or I need to change all reported format error by isort module.

Best

@jiaqiw09
Copy link
Contributor Author

@DN6 Hi,I am sorry for the inconvenience. I just push a new commit trying to solve code-check problem. Would you mind having a look? And if there are other problems, I can fix it quickly.

Best

@jiaqiw09
Copy link
Contributor Author

@patrickvonplaten Hi, I am sorry for the inconvenience. Would you mind having a check again?

Best

@patrickvonplaten
Copy link
Contributor

Also a gentle ping to @SunMarc , do you think using the DistributedKwargs makes sense here?

@sayakpaul
Copy link
Member

Looks good to me, thank you! But I defer to @SunMarc for the final go ahead here.

@SunMarc SunMarc self-requested a review October 26, 2023 19:23
Copy link
Member

@SunMarc SunMarc left a comment

Choose a reason for hiding this comment

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

LGTM @jiaqiw09 ! Don't forget to run make style. Yes this is how you can pass kwargs related to DDP when using accelerate @patrickvonplaten .

@jiaqiw09
Copy link
Contributor Author

jiaqiw09 commented Oct 27, 2023

@patrickvonplaten @sayakpaul @SunMarc

thanks all for your review. I just do not notice that checks has passed 3 hours ago so i just merge latest main branch into my own branch and make a new commit and push to solve codecheck probelm. i hope it will not be a problem.

best

@sayakpaul sayakpaul merged commit e140c05 into huggingface:main Oct 27, 2023
11 checks passed
@sayakpaul
Copy link
Member

Thank you for your contributions!

kashif pushed a commit to kashif/diffusers that referenced this pull request Nov 11, 2023
…uggingface#5355)

* fix error reported 'find_unused_parameters' running in mutiple GPUs or NPUs

* fix code check of importing module by its alphabetic order

---------

Co-authored-by: jiaqiw <wangjiaqi50@huawei.com>
Co-authored-by: Dhruv Nair <dhruv.nair@gmail.com>
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
…uggingface#5355)

* fix error reported 'find_unused_parameters' running in mutiple GPUs or NPUs

* fix code check of importing module by its alphabetic order

---------

Co-authored-by: jiaqiw <wangjiaqi50@huawei.com>
Co-authored-by: Dhruv Nair <dhruv.nair@gmail.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.

Report error about "find_unused_parameters=True" running in mutiple GPUs
6 participants