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

add new api paddle.Tensor.fill_diagonal_ #34460

Merged
merged 1 commit into from
Aug 17, 2021
Merged

add new api paddle.Tensor.fill_diagonal_ #34460

merged 1 commit into from
Aug 17, 2021

Conversation

zhiboniu
Copy link
Contributor

@zhiboniu zhiboniu commented Jul 28, 2021

PR types

Others

PR changes

APIs

Describe

补充缺失API,新增 api paddle.Tensor.fill_diagonal_
功能:实现对Tensor对角线向量值的修改。

使用示例:
x = paddle.ones((3,3))
x.fill_diagonal_(5)

tensor([[5., 1., 1.],
[1., 5., 1.],
[1., 1., 5.]])

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

Comment on lines 47 to 49
"offset of diagonal, zero means no offset, positive means "
"offset to up-right corner; negtive means offset to "
"bottom-left corner")
Copy link
Contributor

Choose a reason for hiding this comment

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

说明改一下?

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

)DOC");
AddInput("X", "(Tensor) The input tensor.");
AddOutput("Out",
"Tensor, the clipped tensor, with the same shape and data type "
Copy link
Contributor

Choose a reason for hiding this comment

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

“the clipped tensor”的说明在这里有什么特殊意义不,是否去掉?

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 95 to 98
framework::TensorCopy(*xin, ctx.GetPlace(), out);

T *out_data = out->mutable_data<T>(ctx.GetPlace());
Copy link
Contributor

Choose a reason for hiding this comment

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

常规做法是先 out->mutable_data 分配内存后再使用 TensorCopy,这里反过来是否有特殊考虑

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, TensorCopy中也有out->mutable_data,所以其实都可以。不过也已经修改。

Comment on lines +61 to +49
framework::TensorCopy(*xin, ctx.GetPlace(), out);

T* out_data = out->mutable_data<T>(ctx.GetPlace());
Copy link
Contributor

Choose a reason for hiding this comment

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

常规做法是先 out->mutable_data 分配内存后再使用 TensorCopy,这里反过来是否有特殊考虑

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,同上已修改。

if ((i - offset) % strides == 0 && i < wrapsize) {
data[i] = T(0);
} else {
data[i] = static_cast<T>(doutdata[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

反向的逻辑中是否也可以使用TensorCopy 批量实现数据cp

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,已修改。

if ((idx - offset) % strides == 0 && idx < wrapsize) {
dst_data[idx] = T(0);
} else {
dst_data[idx] = const_cast<T&>(src_data[idx]);
Copy link
Contributor

Choose a reason for hiding this comment

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

反向的逻辑中是否也可以使用TensorCopy 批量实现数据cp

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,已修改。

AddOutput("Out",
"Tensor, the output tensor, with the same shape and data type "
"as input(x)");
AddAttr<float>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

value只能是float吗

Copy link
Contributor Author

Choose a reason for hiding this comment

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

以float类型读取python数值,然后填入时转化为实际类型填入。

protected:
void Apply(GradOpPtr<T> retv) const override {
retv->SetType("fill_diagonal_grad");
// retv->SetInput("Out", this->Output("Out"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

注释的代码如无需要就直接删除吧

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

expected_grad = np.array(
[[0, 1, 1], [1, 0, 1], [1, 1, 0]]).astype('float32')

typelist = ['float32', 'float64', 'int32', 'int64']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kernel注册了Bool类型,也加上bool类型的测试

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

jiangjiajun
jiangjiajun previously approved these changes Aug 13, 2021
Copy link
Collaborator

@jiangjiajun jiangjiajun left a comment

Choose a reason for hiding this comment

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

LGTM

jzhang533
jzhang533 previously approved these changes Aug 13, 2021
Copy link
Contributor

@jzhang533 jzhang533 left a comment

Choose a reason for hiding this comment

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

LGTM

XiaoguangHu01
XiaoguangHu01 previously approved these changes Aug 13, 2021
Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LG API

x(Tensor): ``x`` is the original Tensor
value(Scale): ``value`` is the value to filled in x
offset(int,optional): the offset to the main diagonal. Default: 0 (main diagonal).
name(str,optional): Name for the operation (optional, default is None)
Copy link
Contributor

Choose a reason for hiding this comment

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

wrap参数缺少说明

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, 已添加

Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LG API

Copy link
Contributor

@jzhang533 jzhang533 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhiboniu zhiboniu changed the title add api fill_diagonal_inplace add new api paddle.Tensor.fill_diagonal_ Aug 17, 2021
Copy link
Contributor

@jeff41404 jeff41404 left a comment

Choose a reason for hiding this comment

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

LGTM

@jeff41404 jeff41404 merged commit 5de576b into PaddlePaddle:develop Aug 17, 2021
@zhiboniu zhiboniu deleted the develop_fill_diagonal_ branch September 27, 2021 10:02
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.

5 participants