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

[Semi-Auto] SPMD Parallel Rule Base #53863

Merged
merged 46 commits into from
Jun 27, 2023

Conversation

JZ-LIANG
Copy link
Contributor

@JZ-LIANG JZ-LIANG commented May 16, 2023

PR types

New features

PR changes

Others

Description

Pcard-70448

A Experimental new mechanism for parallel operator in semi-auto parallel.

  • the base class and common functions for op spmd_rule, which is used for sharding propagation in semi-auto parallel.
  • data class for sharding computation.
  • a example for matmul spmd rule.

@paddle-bot
Copy link

paddle-bot bot commented May 16, 2023

你的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.

@paddle-bot
Copy link

paddle-bot bot commented May 16, 2023

❌ The PR is not created using PR's template. You can refer to this Demo.
Please use PR's template, it helps save our maintainers' time so that more developers get helped.

@JZ-LIANG JZ-LIANG changed the title [Semi-Auto] SPMD Rule Base [Semi-Auto] SPMD Parallel Rule Base May 31, 2023
void DistTensorSpec::set_shape(const std::vector<int64_t>& shape) {
shape_ = shape;
}
const TensorDistAttr& DistTensorSpec::get_dist_attr() const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const TensorDistAttr& DistTensorSpec::get_dist_attr() const {
const TensorDistAttr& DistTensorSpec::dist_attr() const {

U can skip get in get method, same for others

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 15 to 18
#include <glog/logging.h>

#include "paddle/fluid/distributed/auto_parallel/spmd_rules/common.h"
#include "paddle/fluid/distributed/auto_parallel/spmd_rules/rules.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#include <glog/logging.h>
#include "paddle/fluid/distributed/auto_parallel/spmd_rules/common.h"
#include "paddle/fluid/distributed/auto_parallel/spmd_rules/rules.h"
#include "paddle/fluid/distributed/auto_parallel/spmd_rules/common.h"
#include <glog/logging.h>
#include "paddle/fluid/distributed/auto_parallel/spmd_rules/rules.h"

Put the related header first.


// Merge the DistAttr of input tensors and infer the DistAttr of the output
// tensors from the merged input information. The input are DistAttr and Shape
// (wrapp as DistTensorSpec) of the input tensors (tensors follow the same
Copy link
Contributor

Choose a reason for hiding this comment

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

wrapp -> Wrapped

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
}

TensorDistAttr CopyTensorDistAttrForOutput(
Copy link
Contributor

Choose a reason for hiding this comment

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

Better add a copy constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy constructor is not fit in this case since part of data member would be changed after "copy".

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

@@ -379,6 +379,8 @@ def source_include(header_file_path):
#include "paddle/phi/api/profiler/event_tracing.h"
#include "paddle/phi/api/profiler/supplement_tracing.h"

#include "paddle/fluid/distributed/auto_parallel/spmd_rules/dist_tensor_spec.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

phi需要独立编译使用,不能依赖fluid目录,如果确实需要使用该文件,建议将这个文件加到phi算子库目录下

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 立即 Revert 这个依赖

@JZ-LIANG JZ-LIANG merged commit 6863e2a into PaddlePaddle:develop Jun 27, 2023
24 of 25 checks passed
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.

None yet

5 participants