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.

Sign up for GitHub

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

Jump to bottom

add DDP HPO support for sigopt. only main_process will have HPO, and … #18931

Merged
merged 1 commit into from
Sep 12, 2022
Merged

add DDP HPO support for sigopt. only main_process will have HPO, and … #18931

merged 1 commit into from
Sep 12, 2022

Conversation

sywangyi
Copy link
Contributor

@sywangyi sywangyi commented Sep 8, 2022

…pass argument to other process

Signed-off-by: Wang, Yi A yi.a.wang@intel.com

What does this PR do?

Fixes # (issue)
HPO do not support DDP now. add support in sigopt backend

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,
    Pull Request section?
  • 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.
  • Did you write any new necessary tests?

Who can review?

@sywangyi
Copy link
Contributor Author

sywangyi commented Sep 8, 2022

@sgugger @yao-matrix. please help to review

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Sep 8, 2022 •
edited
Loading

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

sgugger
sgugger reviewed Sep 8, 2022
View reviewed changes
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Thanks for your PR, I left a few comments.
Curious if using to_dict and re-instantiating from the dict could have the use of deepcopy and pickle here.

src/transformers/integrations.py Outdated Show resolved Hide resolved
src/transformers/integrations.py Outdated Show resolved Hide resolved
src/transformers/integrations.py Show resolved Hide resolved
src/transformers/integrations.py Outdated Show resolved Hide resolved
@sywangyi
Copy link
Contributor Author

sywangyi commented Sep 9, 2022

@sgugger thanks for the review. the torch.distributed.broadcast_object_list only support list(each element is pickable) input, does not support dict class

sgugger
sgugger approved these changes Sep 9, 2022
View reviewed changes
Copy link
Collaborator

@sgugger sgugger left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for iterating!
Can you just solve the merge conflicts and address the small comment below? Then we should be good to merge.

src/transformers/integrations.py Outdated Show resolved Hide resolved
@sywangyi
add DDP HPO support for sigopt
305130f 
only main_process will have HPO, and pass argument to other process

Signed-off-by: Wang, Yi A <yi.a.wang@intel.com>
@sgugger sgugger merged commit a86acb7 into huggingface:main Sep 12, 2022
oneraghavan pushed a commit to oneraghavan/transformers that referenced this pull request Sep 26, 2022
@sywangyi @oneraghavan
add DDP HPO support for sigopt (huggingface#18931)
e1b2e5f 
only main_process will have HPO, and pass argument to other process

Signed-off-by: Wang, Yi A <yi.a.wang@intel.com>

Signed-off-by: Wang, Yi A <yi.a.wang@intel.com>
@sywangyi sywangyi deleted the ddp_hpo branch October 21, 2022 12:16
@jasper-lu jasper-lu mentioned this pull request Aug 30, 2023
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgugger sgugger sgugger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sywangyi @HuggingFaceDocBuilderDev @sgugger