CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
[DistDialect] add distributed operation attribute #62201
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
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
28b7331
to
fc5208c
Compare
std::vector<TensorDistAttribute>>; | ||
OperationDistAttrStorage(ParamKey&& param) // NOLINT | ||
: process_mesh(std::get<0>(param)), | ||
inputs(std::get<1>(param)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
建议起名 operand_dist_attrs, result_dist_attrs 吧, 对齐pir 的概念
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
建议起名 operand_dist_attrs, result_dist_attrs 吧, 对齐pir 的概念
done
static OperationDistAttrStorage* Construct(ParamKey&& key) { | ||
return new OperationDistAttrStorage(std::move(key)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
缺少一些接口,比如 fluid 下原实现的 get 方法 (等):
const TensorDistAttr& input_dist_attr(const std::string& name) const {
return input_dist_attrs_.at(name);
}
const TensorDistAttr& output_dist_attr(const std::string& name) const {
return output_dist_attrs_.at(name);
}
这里需要按 pir 逻辑按 index 引索
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
缺少一些接口,比如 fluid 下原实现的 get 方法 (等): const TensorDistAttr& input_dist_attr(const std::string& name) const { return input_dist_attrs_.at(name); } const TensorDistAttr& output_dist_attr(const std::string& name) const { return output_dist_attrs_.at(name); } 这里需要按 pir 逻辑按 index 引索
done
const phi::distributed::ProcessMesh& process_mesh() const { | ||
return mesh_attr().process_mesh(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
缺少调用接口,参考 fluid 下实现,不同operand 或 results 的 get方法
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
缺少调用接口,参考 fluid 下实现,不同operand 或 results 的 get方法
done
os << process_mesh_attr.process_mesh(); | ||
} else { | ||
os << "error_attribute_type"; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
后期补充 打印实现
|
||
EXPECT_EQ(op_attr.inputs()[0], x_dist_attr); | ||
EXPECT_EQ(op_attr.inputs()[1], y_dist_attr); | ||
EXPECT_EQ(op_attr.outputs()[1], out_dist_attr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
单测过于简单,构造一些 op_attr1 != op_attr2 不相等 case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
单测过于简单,构造一些 op_attr1 != op_attr2 不相等 case
done
337bdb7
to
6d3476f
Compare
8c6c0b3
to
649e059
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT
TensorDistAttribute result_dist_attr(uint32_t index) const; | ||
uint32_t num_result_dist_attrs() const; | ||
|
||
const phi::distributed::ProcessMesh& process_mesh() const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OperationDistAttribute 只暴露一个 process_mesh 接口吧,只返回 ProcessMeshAttribute,接口都只返回pir 的类型。如果用户需要 phi 类型的那个底层storage,用户需要自己显式用 ProcessMeshAttribute.data() 去获取。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OperationDistAttribute 只暴露一个 process_mesh 接口吧,只返回 ProcessMeshAttribute,接口都只返回pir 的类型。如果用户需要 phi 类型的那个底层storage,用户需要自己显式用 ProcessMeshAttribute.data() 去获取。
修改完成,返回ProcessMeshAttribute类型的函数名统一修改为process_mesh_attr(), ProcessMesh类型的函数名修改为process_mesh()
|
||
static OperationDistAttribute get( | ||
pir::IrContext* ctx, | ||
ProcessMeshAttribute mesh, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在 get 或者 construct 外层补一个 precondition 的check吧, operand 或 result 的 TensorDistAttribute 里的mesh 为 空 后者 和 TensorDistAttribute 的 mesh 相同。 避免出现构造出不同情况。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在 get 或者 construct 外层补一个 precondition 的check吧, operand 或 result 的 TensorDistAttribute 里的mesh 为 空 后者 和 TensorDistAttribute 的 mesh 相同。 避免出现构造出不同情况。
已添加
EXPECT_EQ(op_attr.process_mesh(), process_mesh); | ||
EXPECT_EQ(op_attr.operand_dist_attrs(), operand_dist_attrs); | ||
EXPECT_EQ(op_attr.operand_dist_attr(0), operand_dist_attrs.at(0)); | ||
EXPECT_EQ(op_attr.operand_dist_attr(1), operand_dist_attrs.at(1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
类似,补一个 expect_ne 的单测,判断成员不相等,属性不相等
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
类似,补一个 expect_ne 的单测,判断成员不相等,属性不相等
已添加
5c68a02
to
b266190
Compare
b266190
to
dfe24a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR types
New features
PR changes
Others
Description
Pcard-67164