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

[API 2.0] Fix api sum:(1)input->x;(2)dim->axis;(3)keep_dim->keepdim #26337

Merged
merged 4 commits into from
Aug 19, 2020
Merged

[API 2.0] Fix api sum:(1)input->x;(2)dim->axis;(3)keep_dim->keepdim #26337

merged 4 commits into from
Aug 19, 2020

Conversation

liym27
Copy link
Contributor

@liym27 liym27 commented Aug 16, 2020 •
edited
Loading

PR types

Breaking changes

PR changes

APIs

Describe

  1. Optimize api paddle.sum for API2.0
  2. Fix a bug: reduce_all should be True when len(axis) == len(x.shape)

image

@paddle-bot-old
Copy link

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

@liym27 liym27 force-pushed the api20_sum branch 4 times, most recently from 4deae95 to c860b76 Compare August 17, 2020 08:39
@wangchaochaohu
Copy link
Contributor

添加动态图的单测 添加tuple支持的单测？

@liym27
[API 2.0] Fix api sum:(1)input->sum;(2)dim->axis;(3)keep_dim->keepdim…
e07690f 
…. test=develop

Change-Id: I5d8092b01131236906bb09bdd039a3ac3b20e0ef
@liym27
Copy link
Contributor Author

liym27 commented Aug 17, 2020 •
edited
Loading

添加动态图的单测 添加tuple支持的单测？

Thanks, done

zhhsplendid
zhhsplendid reviewed Aug 17, 2020
View reviewed changes
Copy link
Member

@zhhsplendid zhhsplendid left a comment

Choose a reason for hiding this comment

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

总体来说LGTM，王超的意见增加dim为tuple的单测可以加一下，这个倒不是你代码可能有问题，这样单测也可以防止未来有人没考虑和犯错。

@liym27
Add unittest when axis is tuple; fix bug when len(axis) == len(x.shap…
204b9a2 
…e). test=develop

Change-Id: I1c357afe0989acefe56a1955010dafac92899e31
@liym27
Copy link
Contributor Author

liym27 commented Aug 17, 2020 •
edited
Loading

总体来说LGTM，王超的意见增加dim为tuple的单测可以加一下，这个倒不是你代码可能有问题，这样单测也可以防止未来有人没考虑和犯错。

好的，已添加axis为tuple的单测。
同时修复了一个bug: len(axis) == len(x.shape)，reduce_all参数应该为True

@liym27
add type(None) for attr(axis). test=develop
1e77d45 
Change-Id: I5aa38c17871b35081ffa6d1d3498a681503ad0a2
wawltor
wawltor reviewed Aug 18, 2020
View reviewed changes
python/paddle/tensor/math.py Outdated Show resolved Hide resolved
python/paddle/tensor/math.py Show resolved Hide resolved
@liym27
fix api doc according to reviews. test=develop
8d74e5d 
Change-Id: I6c98e1f95acca108b4b045e320db0be2b085a75a
wawltor
wawltor approved these changes Aug 18, 2020
View reviewed changes
Copy link
Contributor

@wawltor wawltor left a comment

Choose a reason for hiding this comment

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

LGTM

wangchaochaohu
wangchaochaohu approved these changes Aug 18, 2020
View reviewed changes
Copy link
Contributor

@wangchaochaohu wangchaochaohu left a comment

Choose a reason for hiding this comment

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

LGTM

zhhsplendid
zhhsplendid approved these changes Aug 18, 2020
View reviewed changes
Copy link
Member

@zhhsplendid zhhsplendid left a comment

Choose a reason for hiding this comment

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

LGTM

@liym27 liym27 changed the title [API 2.0] Fix api sum:(1)input->sum;(2)dim->axis;(3)keep_dim->keepdim [API 2.0] Fix api sum:(1)input->x;(2)dim->axis;(3)keep_dim->keepdim Aug 19, 2020
@liym27 liym27 merged commit 6cd67a8 into PaddlePaddle:develop Aug 19, 2020
@liym27 liym27 mentioned this pull request Aug 20, 2020
Merged
@liym27 liym27 deleted the api20_sum branch December 9, 2020 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhhsplendid zhhsplendid zhhsplendid approved these changes

@wangchaochaohu wangchaochaohu wangchaochaohu approved these changes

@wawltor wawltor wawltor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@liym27 @wangchaochaohu @zhhsplendid @wawltor