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

update dygraph auto parallel API interface. #59059

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

wuhuachaocoding
Copy link
Contributor

@wuhuachaocoding wuhuachaocoding commented Nov 16, 2023

PR types

New features

PR changes

APIs

Description

update dygraph auto parallel API interface.
Pcard-73145

@CLAassistant
Copy link

CLAassistant commented Nov 16, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


wuhuachao seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Member

@ForFishes ForFishes left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@LiYuRio LiYuRio left a comment

Choose a reason for hiding this comment

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

LGTM

@wuhuachaocoding
Copy link
Contributor Author

@jzhang533

Copy link
Contributor

@jzhang533 jzhang533 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@zhiqiu zhiqiu left a comment

Choose a reason for hiding this comment

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

LGTM


from paddle.base.core import Partial, Placement, ReduceType, Replicate, Shard

__all__ = ["ReduceType", "Placement", "Replicate", "Shard", "Partial"]
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. ReduceType 和其他类型命名方式不太一样,多了个Type的后缀,是否需要统一?
  2. placement_type 包含 Placement,这两个概念有包含关系,Placement是一种什么样的placement_type呢?
  3. 这几个API不需要放到__all__列表,根据调用方式,只放到paddle.distributed.init.py的__all__列表就行

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,下一个PR修改

@ForFishes ForFishes merged commit 33854f2 into PaddlePaddle:develop Nov 27, 2023
27 of 28 checks passed
SecretXV pushed a commit to SecretXV/Paddle that referenced this pull request Nov 28, 2023
Co-authored-by: wuhuachao <wuhuachao@baidu.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.

7 participants