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

【Hackathon 5th No.4】为 Paddle 新增 masked_scatter API RFC #629

Merged
merged 24 commits into from Oct 23, 2023

Conversation

ranchongzhi
Copy link
Contributor

No description provided.

@paddle-bot
Copy link

paddle-bot bot commented Sep 15, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请检查PR提交格式和内容是否完备,具体请参考示例模版
Your PR has been submitted. Thanks for your contribution!
Please check its format and content. For this, you can refer to Template and Demo.

@ranchongzhi
Copy link
Contributor Author

@zoooo0820 老师我重新提交了一版RFC,麻烦您抽空看看

@ranchongzhi
Copy link
Contributor Author

@zoooo0820 麻烦review一下

@ranchongzhi
Copy link
Contributor Author

@zoooo0820 老师您看还有问题吗


### masked_scatter的初步实现

初步实现如下:
Copy link
Contributor

Choose a reason for hiding this comment

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

这部分初步实现的想法可以搬运到下面API实现方案

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

return paddle.index_put(x, indexs, value.flatten()[:mask.sum()])
else:
"""
经过测试,静态图模式下(当x的shape中含有-1)broasdcast_to广播操作失效。但是可以借助乘法来间接实现广播效果
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


Tensor.masked_scatter(mask, value)

Tensor.masked_scatter_(mask, value)
Copy link
Contributor

Choose a reason for hiding this comment

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

函数签名中需要补充name=None

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 Author

Choose a reason for hiding this comment

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

@zoooo0820 已全部修改

@ranchongzhi
Copy link
Contributor Author

@zoooo0820 @luotao1 麻烦帮忙看看还有问题吗

Copy link
Contributor

@zoooo0820 zoooo0820 left a comment

Choose a reason for hiding this comment

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

LGTM

@zoooo0820 zoooo0820 merged commit a233a30 into PaddlePaddle:master Oct 23, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants