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 No.23】为 Paddle 新增 paddle.incubate.sparse.is_same_size 稀疏 API #184

Merged
merged 5 commits into from
Aug 8, 2022

Conversation

gsq7474741
Copy link
Contributor

No description provided.

@paddle-bot
Copy link

paddle-bot bot commented Jul 12, 2022

你的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.

Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

  1. API名 paddle.is_same_shape, 建议API实现放到 python/paddle/tensor/attribute.py 里,且动态图下直接类函数即可
  2. 动态图下:建议在 paddle/fluid/pybind/eager_method.cc 中实现,作为TensorObject的类成员函数,对比self.tensor.shape是否一致
  3. 目前不支持静态图,加装饰器@dygraph_only,描述不支持原因:静态图机制目前无法使用SparseCooTensor/SparseCsrTensor
  4. 单侧补充dense、coo、csr不同类型的9种组合情形

@zhwesky2010
Copy link
Contributor

另外需要处理一下冲突

@gsq7474741
Copy link
Contributor Author

  1. API名 paddle.is_same_shape, 建议API实现放到 python/paddle/tensor/attribute.py 里,且动态图下直接类函数即可
  2. 动态图下:建议在 paddle/fluid/pybind/eager_method.cc 中实现,作为TensorObject的类成员函数,对比self.tensor.shape是否一致
  3. 目前不支持静态图,加装饰器@dygraph_only,描述不支持原因:静态图机制目前无法使用SparseCooTensor/SparseCsrTensor
  4. 单侧补充dense、coo、csr不同类型的9种组合情形

要判断的是shape还是size啊

@gsq7474741
Copy link
Contributor Author

image
任务要求是这样写的,您看要不要修改一下任务 @zhouwei25

@zhwesky2010
Copy link
Contributor

zhwesky2010 commented Jul 26, 2022

按照后面review的来吧,这个任务是我写的最初版,后面详细review时会有一些细节改动

@gsq7474741
Copy link
Contributor Author

已更新 @zhouwei25

Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

你好,还需要微调下目录和API签名。整体没有其他问题了

@@ -0,0 +1,88 @@
# paddle.is_same_shape 设计文档
Copy link
Contributor

Choose a reason for hiding this comment

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

你好,经过一些考虑,还是在python/paddle/incubate/sparse/multiary.py下实现吧,与dense区分下,API签名:paddle.incubate.sparse.is_same_shape。


## API实现方案

参考numpy实现,对比self.tensor.shape是否一致
Copy link
Contributor

Choose a reason for hiding this comment

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

动态图实现:在 paddle/fluid/pybind/eager_method.cc 中实现,作为TensorObject的类成员函数,对比self.tensor.shape是否一致。
静态图实现:目前sparse系列暂不支持静态图


## 命名与参数设计

在 python/paddle/tensor/attribute.py 中新增api,
Copy link
Contributor

Choose a reason for hiding this comment

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

python/paddle/incubate/sparse/multiary.py

@gsq7474741
Copy link
Contributor Author

你好,还需要微调下目录和API签名。整体没有其他问题了

已更新

Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants