-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[PIR] Refine op yaml system #59364
[PIR] Refine op yaml system #59364
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
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
|
||
IrSelectedRows::IrSelectedRows(const IrSelectedRows& other) { | ||
dims_ = other.dims(); | ||
dtype_ = other.dtype(); |
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.
下面这几个构造函数是不是都可以用=default 就可以了。因为成员里没有指针对象,默认的应该够用?
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.
感谢建议,单独 pr 完善一下~
return *this; | ||
} | ||
|
||
int64_t IrSelectedRows::numel() const { return phi::product(dims_); } |
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.
这里会出现 [128, 3, -1, -1]的场景么?或者说这里的ddim都是非负数了已经
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.
可能会出现,IrTensor 和 IrSelectRows 仅会PIR组网的 infer meta 阶段使用,全局搜索了一下框架目前所有的 infer_meta 函数,共有2个使用到 numel 的地方:NanmedianInferMeta、FullWithTensorInferMeta,但这两个函数并未对这种情况做处理。目前执行未发现错误应该是因为执行期间还会执行一次 infer_meta.
后续我单独 pr 对IrTensor 和 IrSelectRows 的 numel 接口进行完善,在存在-1的情况下返回-1,此外再对NanmedianInferMeta、FullWithTensorInferMeta 这两个 infer_meta 的实现进行完善
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.
approve for key composite
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
本 PR 对 PIR 的算子 yaml 定义开展了系统的重构:
1、当前 PIR 算子 yaml 定义体系及存在的问题:
目前 PIR 的算子定义分布在 6 个 yaml 文件中(不区分前反向),包括:
当前存在问题包括:
基于上述问题,我们对 PIR 的算子 yaml 进行了如下的规范:
2、重构后算子 yaml 定义体系:
将 phi/legacy_ops.yaml 定义的算子移动到 pir/dialect/ops.yaml,由此,PIR下算子定义包括:
3、PIR 下,参照如下流程图完成算子定义位置的确定:
Pcard-67164