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

Bugfix: kAddTo support for swap_axes. #9541

Merged
merged 1 commit into from
Jan 24, 2018

Conversation

tdomhan
Copy link
Contributor

@tdomhan tdomhan commented Jan 24, 2018

Description

With this PR the swapaxes operator no longer ignores the req argument. Previously this silently lead to wrong gradients.

Checklist

Essentials

  • Passed code style checking (make lint)
  • Changes are complete (i.e. I finished coding on this PR)
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

@KellenSunderland
Copy link
Contributor

@eric-haibin-lin Any chance we can get this in the next release?

@KellenSunderland
Copy link
Contributor

@tdomhan Could you amend the commit and add

Co-authored-by: Hetzel <ahhetzel@amazon.com>

at the bottom?

Co-authored-by: Asmus Hetzel <ahhetzel@amazon.com>.
@tdomhan
Copy link
Contributor Author

tdomhan commented Jan 24, 2018

yeah of course. I also wouldn't mind to close the PR and update the other PR. Just whatever solution gets the fix into the next release works for me :)

@asmushetzel
Copy link
Contributor

@piiswrong
Somehow I forgot about the pending #9495 , sorry. This here is indeed the suitable fix. I will close 9495.
That definitely should go into next release

@sxjscience
Copy link
Member

@piiswrong
Copy link
Contributor

I'm merging this because code freeze is today. Let's update tests later.

@marcoabreu Please track this. We should actually test for addto for ALL operators automatically.

@marcoabreu
Copy link
Contributor

marcoabreu commented Jan 24, 2018

Tracked at #9497

@tdomhan would you mind adding a test in a second PR? It would be great if we could test this ASAP.

@piiswrong piiswrong merged commit 5f9dd2c into apache:master Jan 24, 2018
yuxiangw pushed a commit to yuxiangw/incubator-mxnet that referenced this pull request Jan 25, 2018
Co-authored-by: Asmus Hetzel <ahhetzel@amazon.com>.
@tdomhan tdomhan deleted the my_swap_axis_fix branch January 25, 2018 15:47
@tdomhan
Copy link
Contributor Author

tdomhan commented Jan 25, 2018

thanks for merging! I will try to create an appropriate test based on pointers from @sxjscience.

rahul003 pushed a commit to rahul003/mxnet that referenced this pull request Jun 4, 2018
Co-authored-by: Asmus Hetzel <ahhetzel@amazon.com>.
zheng-da pushed a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 28, 2018
Co-authored-by: Asmus Hetzel <ahhetzel@amazon.com>.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants