Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

Fix the error of gradient of np.pad#19044

Merged
sxjscience merged 7 commits intoapache:masterfrom
ryanwentaoxu:master
Sep 1, 2020
Merged

Fix the error of gradient of np.pad#19044
sxjscience merged 7 commits intoapache:masterfrom
ryanwentaoxu:master

Conversation

@ryanwentaoxu
Copy link
Contributor

@ryanwentaoxu ryanwentaoxu commented Aug 30, 2020

@mxnet-bot
Copy link

mxnet-bot commented Aug 30, 2020

Hey @cassinixu , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [sanity, unix-gpu, centos-cpu, edge, clang, unix-cpu, windows-cpu, centos-gpu, windows-gpu, miscellaneous, website]

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

sxjscience
sxjscience previously requested changes Aug 30, 2020
View reviewed changes
Copy link
Member

@sxjscience sxjscience 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.

The gradient of padding is wrong in a very general way.

@sxjscience sxjscience dismissed their stale review August 30, 2020 21:12

Gradient of the the constant padding case is fixed.

sxjscience
sxjscience reviewed Aug 30, 2020
View reviewed changes
@ryanwentaoxu
Copy link
Contributor Author

ryanwentaoxu commented Aug 31, 2020

@mxnet-bot run ci [unix-cpu, windows-cpu]

@mxnet-bot
Copy link

mxnet-bot commented Aug 31, 2020

Jenkins CI successfully triggered : [windows-cpu, unix-cpu]

@ryanwentaoxu
Copy link
Contributor Author

ryanwentaoxu commented Aug 31, 2020

@mxnet-bot run ci [centos-cpu]

@mxnet-bot
Copy link

mxnet-bot commented Aug 31, 2020

Jenkins CI successfully triggered : [centos-cpu]

sxjscience
sxjscience reviewed Aug 31, 2020
View reviewed changes
sxjscience
sxjscience reviewed Aug 31, 2020
View reviewed changes
sxjscience
sxjscience reviewed Aug 31, 2020
View reviewed changes
@sxjscience sxjscience merged commit af467d2 into apache:master Sep 1, 2020
@sxjscience sxjscience mentioned this pull request Sep 1, 2020
Closed
@sxjscience
Copy link
Member

sxjscience commented Sep 7, 2020

@ciyongch @szha Would you know if it's possible to backport the bug-fix to 1.7.x and 1.x?

@ciyongch
Copy link
Contributor

ciyongch commented Sep 7, 2020

Hi @sxjscience , thanks for reaching us for the issue, I'm afraid it's not able to be part of official 1.7.0 release as it's almost completed. But I think this fix should be fine to be backported into both 1.7.x and 1.x branches, and it's likely to be included in the next release (version: 1.8.0) which lead by @samskalicky .

@sxjscience sxjscience mentioned this pull request Sep 7, 2020
Open
sxjscience added a commit to sxjscience/mxnet that referenced this pull request Sep 17, 2020
@ryanwentaoxu @sxjscience
Fix the error of gradient of np.pad (apache#19044)
b0f8ea9 
* pad grad modified

* Fix pad grad error

* modify pad constant backward

* Fix test error

* Fix test error

* Fix kAddTo supported

* Add test for grad_req='add'

Co-authored-by: Xingjian Shi <[email protected]>
@sxjscience sxjscience mentioned this pull request Sep 17, 2020
Merged
6 tasks
samskalicky pushed a commit that referenced this pull request Sep 17, 2020
@sxjscience @ryanwentaoxu
Fix the error of gradient of np.pad (#19044) (#19167)
039fef9 
* pad grad modified

* Fix pad grad error

* modify pad constant backward

* Fix test error

* Fix test error

* Fix kAddTo supported

* Add test for grad_req='add'

Co-authored-by: Xingjian Shi <[email protected]>

Co-authored-by: Wentao Xu <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@sxjscience sxjscience sxjscience left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Numpy][Bug] The gradient of np.pad is wrong!

4 participants

@ryanwentaoxu @mxnet-bot @sxjscience @ciyongch