[Pten] Support optional param for C++ API #39760
Conversation
|
Thanks for your contribution! |
| MetaTensor* dy, | ||
| MetaTensor* dz) { | ||
| if (dx) { | ||
| dx->share_meta(x); |
There was a problem hiding this comment.
The actual behaviour is "copy_meta" in stead of so-called "share_meta", which is misleading. Strongly suggest that we should adjust this interface name in another PR.
paddle/phi/api/lib/api_utils.h
Outdated
| @@ -31,6 +31,14 @@ inline std::shared_ptr<phi::DenseTensor> TensorToDenseTensor( | |||
| return std::dynamic_pointer_cast<phi::DenseTensor>(tensor.impl()); | |||
| } | |||
|
|
|||
| inline paddle::optional<const phi::DenseTensor&> TensorToDenseTensor( | |||
There was a problem hiding this comment.
paddle::optional<T&> doesn't seem to own the referenced object:
Which means the returned object "paddle::optional<const phi::DenseTensor&>" is under the risk of having dangling reference once the input "tensor" is destructed.
If this function is only used in generated C++ API, and we're confident about the lifetime management, can we at least add some comments to warn the risk of using this interface?
There was a problem hiding this comment.
想了一下，这里直接返回std::shared_ptr<phi::DenseTensor>可能会更好一些，在形式上也更统一，是否为空的判断逻辑可以在自动生成的API内部代码里处理
paddle/phi/api/lib/api_utils.h
Outdated
| @@ -49,12 +57,28 @@ inline std::shared_ptr<phi::SelectedRows> TensorToSelectedRows( | |||
| return std::dynamic_pointer_cast<phi::SelectedRows>(tensor.impl()); | |||
| } | |||
|
|
|||
| inline paddle::optional<const phi::SelectedRows&> TensorToSelectedRows( | |||
| /* ----------------- for infer_meta --------------------- */ | ||
|
|
||
| inline phi::MetaTensor MakeMetaTensor(const phi::DenseTensor& tensor) { | ||
| return phi::MetaTensor(tensor); | ||
| } | ||
|
|
||
| inline paddle::optional<phi::MetaTensor> MakeMetaTensor( |
There was a problem hiding this comment.
It seems that sometimes the function returns "paddle::optional", while othertimes it returns "paddle::optional<T&>". May I ask what the design is, like what determines the return type?
There was a problem hiding this comment.
-
由于这里需要创建一个新的MetaTensor出来，如果直接返回
optional<const T&>类型在函数退出后对象会被析构，如果直接返回对象，生成的代码在写法上存在问题，综合考虑这里目前返回optional<phi::MetaTensor>最为合适。 -
目前底层Kernel的参数形式为
optional<const DenseTensor&>，InferMeta的参数形式需要与Kernel保持一致，所以在调用InferMeta和Kernel前都需要将参数统一转为paddle::optional<const T&>的形式
| @@ -69,6 +93,14 @@ inline phi::MetaTensor MakeMetaTensor(const phi::SelectedRows& tensor) { | |||
| return phi::MetaTensor(tensor); | |||
| } | |||
|
|
|||
| inline paddle::optional<phi::MetaTensor> MakeMetaTensor( | |||
zyfncg
dismissed stale reviews from MingMingShangTian and chenwhql
via
d773761
PR types
New features
PR changes
APIs
Describe
C++ API支持原Op体系下的Dispensable机制（即输入为可选），可通过yaml文件中optional项进行配置（如
optiona : x, y，具体示例可参考matmul_double_grad配置）。