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.31】为 Paddle 新增 column_stack / row_stack / dstack / hstack / vstack API #684

Merged
merged 2 commits into from
Nov 3, 2023

Conversation

megemini
Copy link
Contributor

@megemini megemini commented Oct 6, 2023

PR types

Others

PR changes

Docs

Description

【Hackathon 5th No.31】为 Paddle 新增 column_stack / row_stack / dstack / hstack / vstack API

请评审!

@paddle-bot
Copy link

paddle-bot bot commented Oct 6, 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.

- `paddle.vstack`,作为独立的函数调用
- `Tensor.vstack`,作为 Tensor 的方法使用

**疑问** : 这些接口为什么会有 `Tensor.XXX` 的方法?一般堆叠都是对多个 Tensor 的操作,另外,`PyTorch`、`Numpy` 也没有类似的使用方法,还请确认一下。
Copy link
Contributor

Choose a reason for hiding this comment

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

不需要类方法,5个Tensor.*都移除

添加 python 上层接口:

- `paddle.column_stack(x, name=None)`
- `Tensor.column_stack` (有疑问)
Copy link
Contributor

Choose a reason for hiding this comment

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

类方法都去掉


- **维度测试**
- Paddle API 支持的最低维度为 0 维,单测中应编写相应的 0 维尺寸测试 case

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.

OK,需要有异常测试的,我尽快更新一下 ~ 谢谢!

需要保证前向计算的精度正确性,通过 numpy 实现的函数的对比结果

- **维度测试**
- Paddle API 支持的最低维度为 0 维,单测中应编写相应的 0 维尺寸测试 case
Copy link
Contributor

Choose a reason for hiding this comment

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

是的,需要注意0维的问题

@megemini
Copy link
Contributor Author

@zhwesky2010

非常抱歉拖了多日才更新,此次更新:

  • 移除 Tensor.xxx 相关接口
  • 增加单测项目,以及异常测试

请评审~ 非常感谢!

@megemini megemini requested a review from zhwesky2010 October 28, 2023 11:25
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.

3 participants