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.

Sign up for GitHub

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

Jump to bottom

【Hackathon No.27】为 Paddle 新增 frac 数学计算API #41226

Merged
merged 5 commits into from
Apr 12, 2022
Merged

【Hackathon No.27】为 Paddle 新增 frac 数学计算API #41226

merged 5 commits into from
Apr 12, 2022

Conversation

Asthestarsfalll
Copy link
Contributor

@Asthestarsfalll Asthestarsfalll commented Mar 31, 2022 •
edited
Loading

PR types

Others

PR changes

APIs

Describe

解决了issue：#40306
增加了paddle.frac和Tensor.frac。具体地，paddle.frac可以返回输入tensor的小数部分。
设计文档：PaddlePaddle/community#51
中文文档：PaddlePaddle/docs#4539

@paddle-bot-old paddle-bot-old bot added contributor External developers status: proposed labels Mar 31, 2022
@paddle-bot-old
Copy link

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@Asthestarsfalll Asthestarsfalll mentioned this pull request Apr 1, 2022
Closed
@dingjiaweiww
Copy link
Contributor

PR-CI-Mac-Python3和PR-CI-Inference 是需要通过的噢~

@Asthestarsfalll
Copy link
Contributor Author

PR-CI-Mac-Python3和PR-CI-Inference 是需要通过的噢~

你好，已经通过了~

luotao1
luotao1 reviewed Apr 6, 2022
View reviewed changes
python/paddle/fluid/tests/unittests/test_frac_api.py Outdated
out_ref = ref_frac(self.x_np)
self.assertTrue(np.allclose(out_ref, res))

def test__dygraph_api(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

test__dygraph_api->test_dygraph_api

python/paddle/fluid/tests/unittests/test_frac_api.py
x = paddle.to_tensor(self.x_np)
out = paddle.frac(x)
out_ref = ref_frac(self.x_np)
self.assertTrue(np.allclose(out_ref, out.numpy()))
Copy link
Contributor

Choose a reason for hiding this comment

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

根据验收标准，请多补充下测试case。

  • 目前只测了float32，请补充下int32, int64, float64的case
  • 需要有test_errors异常的测试，可以测一下dtype不在上述4个类型中的情况

@Asthestarsfalll
Copy link
Contributor Author

@luotao1 你好，已修改。

luotao1
luotao1 previously approved these changes Apr 8, 2022
View reviewed changes
Copy link
Contributor

@luotao1 luotao1 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

主体功能LGTM，还需要过一下static-check流水线，同时增加中文文档

luotao1
luotao1 reviewed Apr 8, 2022
View reviewed changes
python/paddle/tensor/__init__.py Outdated
@@ -452,6 +453,7 @@
'digamma',
'diagonal',
'trunc',
'frac'
Copy link
Contributor

Choose a reason for hiding this comment

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


2022-04-07 21:45:56 API Difference is: 
2022-04-07 21:45:56 - paddle.Tensor.bitwise_and (ArgSpec(args=['x', 'y', 'out', 'name'], varargs=None, varkw=None, defaults=(None, None), kwonlyargs=[], kwonlydefaults=None, annotations={}), ('document', '16c92f81b99632969af31978cf06dcbd'))
2022-04-07 21:45:56 + paddle.bitwise_and (ArgSpec(args=['x', 'y', 'out', 'name'], varargs=None, varkw=None, defaults=(None, None), kwonlyargs=[], kwonlydefaults=None, annotations={}), ('document', '16c92f81b99632969af31978cf06dcbd'))

static-check流水线显示：https://xly.bce.baidu.com/paddlepaddle/paddle/newipipe/detail/5338157/job/13849637
这里少了一个逗号。

luotao1
luotao1 reviewed Apr 8, 2022
View reviewed changes
python/paddle/__init__.py
@@ -263,6 +263,7 @@
from .tensor.math import fmin # noqa: F401
from .tensor.math import inner # noqa: F401
from .tensor.math import outer # noqa: F401
from .tensor.math import frac # noqa: F401
Copy link
Contributor

Choose a reason for hiding this comment

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

需要在下面的__all__列表里加上frac

@luotao1
Copy link
Contributor

luotao1 commented Apr 8, 2022 

2022-04-08 15:50:15 fatal: refusing to merge unrelated histories
2022-04-08 15:50:15 PADDLE DOCKER BUILD md5=
2022-04-08 15:50:15 check docker md5 fail !
2022-04-08 15:50:15 the build(7c7a664b04d74a75970fc8e31d7dd17e) state is BUILD_CODE_FAIL

CI错误可以试一下merge develop，https://github.com/Asthestarsfalll/Paddle/tree/frac 落后212个

@Asthestarsfalll
Copy link
Contributor Author

CI错误可以试一下merge develop，https://github.com/Asthestarsfalll/Paddle/tree/frac 落后212个

多谢提醒！

@Asthestarsfalll
Copy link
Contributor Author

已添加中文文档

@dingjiaweiww
Copy link
Contributor

PR-CI-Windows和PR-CI-Windows-Inference这两个CI是需要通过的哦~

@Asthestarsfalll
Copy link
Contributor Author

@dingjiaweiww 你好，已通过

@luotao1 luotao1 requested a review from DDDivano April 12, 2022 06:19
DDDivano
DDDivano approved these changes Apr 12, 2022
View reviewed changes
Copy link
Contributor

@DDDivano DDDivano left a comment

Choose a reason for hiding this comment

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

LGTM

@dingjiaweiww dingjiaweiww self-requested a review April 12, 2022 09:40
@luotao1 luotao1 merged commit ce5e119 into PaddlePaddle:develop Apr 12, 2022
@Asthestarsfalll Asthestarsfalll deleted the frac branch May 26, 2022 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luotao1 luotao1 luotao1 approved these changes

@DDDivano DDDivano DDDivano approved these changes

@dingjiaweiww dingjiaweiww dingjiaweiww approved these changes

@lanxianghit lanxianghit lanxianghit approved these changes

Assignees

@luotao1 luotao1

@DDDivano DDDivano

Labels
contributor External developers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Asthestarsfalll @dingjiaweiww @luotao1 @DDDivano @lanxianghit