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

Unify ProcessGroupNCCL APIs underlying implementation #48163

Merged
merged 3 commits into from
Nov 21, 2022

Conversation

HermitSun
Copy link
Contributor

@HermitSun HermitSun commented Nov 18, 2022

PR types

Others

PR changes

APIs

Describe

由于集合通信ProcessGroupNCCL底层接口的实现高度重合,在此统一接口实现,并规范接口参数,为动静统一时静态图传入的 const input 提供支持。

@paddle-bot
Copy link

paddle-bot bot commented Nov 18, 2022

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@HermitSun HermitSun changed the title Unify collective communication C++ APIs underlying implementation Unify collective communication NCCL C++ APIs underlying implementation Nov 18, 2022
@HermitSun HermitSun changed the title Unify collective communication NCCL C++ APIs underlying implementation Unify ProcessGroupNCCL APIs underlying implementation Nov 18, 2022
phi::DenseTensor* tensor,
int rank,
std::shared_ptr<ProcessGroup::Task> ProcessGroupNCCL::NCCLEnv(
const phi::DenseTensor& tensor,
Copy link
Contributor

Choose a reason for hiding this comment

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

如果想要这个语义的话,不建议传入一个tensor,还不如place和holder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

当时是考虑到万一后面还有别的参数,tensor 里包含的信息比较多

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

@sljlp sljlp merged commit 8841022 into PaddlePaddle:develop Nov 21, 2022
@HermitSun HermitSun deleted the collective-p2p branch November 21, 2022 02:55
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.

3 participants