-
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
[API/OP]Add a new API paddle.diagonal #33586
Conversation
Thanks for your contribution! |
using framework::OperatorWithKernel::OperatorWithKernel; | ||
|
||
void InferShape(framework::InferShapeContext *ctx) const override { | ||
PADDLE_ENFORCE_EQ( |
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.
Better use OP_INOUT_CHECK
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.
Done, thanks!
platform::errors::NotFound("Output of DiagonalOp is not found.")); | ||
|
||
int offset_ = ctx->Attrs().Get<int>("offset"); | ||
int dim1 = ctx->Attrs().Get<int>("axis1"); |
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.
Maybe dim1
->axis1
is better, to be consistent with op's 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.
Done, thanks!
PADDLE_ENFORCE_NE(dim1_, dim2_, | ||
platform::errors::InvalidArgument( | ||
"The dimensions should not be identical " | ||
"%ld vs %ld.", |
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.
%d for int
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.
Done, thanks!
auto dim1_size = out_dims[dim1_]; | ||
auto dim2_size = out_dims[dim2_]; | ||
out_dims.erase(out_dims.begin() + std::max(dim1_, dim2_)); | ||
out_dims.erase(out_dims.begin() + std::min(dim1_, dim2_)); |
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.
add some comments
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.
Done, thanks!
"(Tensor) The partial view of input with the its diagonal elements."); | ||
AddAttr<int>( | ||
"offset", | ||
R"DOC((int, default 0), offset of the diagonal from the main diagonal. Can be both positive and negative. Defaults: 0. |
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.
Defaults -> 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.
Done, thanks!
@@ -0,0 +1,362 @@ | |||
/* Copyright (c) 2019 PaddlePaddle Authors. All Rights Reserved. |
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.
2019 -> 2021
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.
Done, thanks!
cudaMalloc(reinterpret_cast<void**>(&input_stride), | ||
input_stride_size * sizeof(int64_t)); | ||
cudaMemcpy(reinterpret_cast<void*>(input_stride), | ||
reinterpret_cast<void*>(host_input_stride), | ||
input_stride_size * sizeof(int64_t), cudaMemcpyHostToDevice); |
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.
Do not use cudaMemcpy/cudaMalloc in op kernel directly. Firstly, try to use less memory as possible, i.e., do not malloc extra memory if can be avoid by refine the algorithm. Secondly, If temporary memory is exactly needed, use temp Tensor, and call tensor.mutable_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.
Replace cudaMalloc/cudaMemcpy by TensorFormVector in cudaKernel and cudaGradKernel.
…s2), set the diagonal dim is 0
@@ -355,6 +356,8 @@ | |||
'shape', | |||
'real', | |||
'imag', | |||
'digamma', |
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.
这行要删除,和362重复
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.
Done, thanks!
python/paddle/tensor/math.py
Outdated
# [[0.50427347, 0.78351408, 0.00833563], | ||
# [0.36932808, 0.83527362, 0.49408615]]]) | ||
|
||
out = paddle.diagonal(x) |
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.
需要补充以下几个示例:
- axis1 = 1的示例
- offset != 0,axis1 = 1 的示例
- offset != 0、axis1 = 1、axis2 != 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.
Done, thanks!
python/paddle/tensor/math.py
Outdated
name (str, optional): Normally there is no need for user to set this property. For more information, please refer to :ref:`api_guide_Name`. Default: None. | ||
|
||
Returns: | ||
Tensor: the output data type is the same as input data 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.
少了,输入 Tensor
在指定二维平面的局部视图 这一句
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.
Done, thanks!
python/paddle/tensor/math.py
Outdated
if in_dygraph_mode(): | ||
return core.ops.diagonal(x, 'offset', offset, 'axis1', axis1, 'axis2', axis2) | ||
|
||
if not in_dygraph_mode(): |
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.
这行代码是否多余?
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.
已删除,谢谢!
python/paddle/tensor/math.py
Outdated
|
||
""" | ||
inputs = {'Input': [x]} | ||
attrs = {'offset': offset, 'axis1': axis1, 'axis2': axis2} |
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.
这两个操作为什么放在动态图调用之前?
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.
该段代码没有什么意义,已删除,谢谢!
python/paddle/tensor/math.py
Outdated
- If offset < 0, it is below the main diagonal. | ||
|
||
Args: | ||
x(Tensor): The input tensor x. Must be at least 2-dimensional. The input data type should be int32, int64, float32, float64. |
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.
应该考虑支持bool、fp16等更多类型
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.
已添加支持,谢谢!
python/paddle/tensor/math.py
Outdated
- If offset < 0, it is below the main diagonal. | ||
|
||
Args: | ||
x(Tensor): The input tensor x. Must be at least 2-dimensional. The input data type should be int32, int64, float32, float64, float16, bool. |
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.
x(Tensor): The input tensor x. Must be at least 2-dimensional. The input data type should be int32, int64, float32, float64, float16, bool. | |
x(Tensor): The input tensor x. Must be at least 2-dimensional. The input data type should be bool, int32, int64, float16, float32, float64. |
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.
Done, thanks!
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
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
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
APIs
Describe
This OP computes the diagonals of the input tensor x.
If x is 2D, returns the diagonal.
If x has larger dimensions, diagonals be taken from the 2D planes specified by axis1 and axis2.
By default, the 2D planes formed by the first and second axis of the input tensor x.
The argument offset determines where diagonals are taken from input tensor x:
- If offset = 0, it is the main diagonal.
- If offset > 0, it is above the main diagonal.
- If offset < 0, it is below the main diagonal.
Args:
if attr
axis1
oraxis2
is over the dims if imput Tensor, it will return a OutOfRange error.if attr
offset
is over the range of [axis1, axis2], it will set the new dim as 0.Returns:
Tensor: the output data type is the same as input data type.
Examples:
API doc: