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

add reverse option to ndarray inplace reshape #10767

Merged
merged 2 commits into from
May 3, 2018

Conversation

szha
Copy link
Member

@szha szha commented May 1, 2018

Description

add reverse option to ndarray inplace reshape

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • add reverse option to ndarray inplace reshape

@piiswrong
Copy link
Contributor

I think this is over complicated. besides the symbolic version doesn't support it

@szha
Copy link
Member Author

szha commented May 1, 2018

It's used a lot in attention, and the in-place option can make a huge difference in saving memory.

@szha
Copy link
Member Author

szha commented May 1, 2018

Symbol version reshape is a pass-through to the reshape operator, so reverse is already supported. https://mxnet.incubator.apache.org/versions/master/api/python/symbol/symbol.html?highlight=reshape#mxnet.symbol.Symbol.reshape

@@ -663,6 +663,7 @@ MXNET_DLL int MXNDArrayReshape(NDArrayHandle handle,
MXNET_DLL int MXNDArrayReshape64(NDArrayHandle handle,
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you check that this does not break our other frontend APIs? In this PR, you only adapter the Python version.

Copy link
Member Author

@szha szha May 2, 2018

Choose a reason for hiding this comment

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

I added this recently, and I haven't seen it used in other API in any PR.

@szha
Copy link
Member Author

szha commented May 2, 2018

ping @piiswrong

else:
assert not kwargs,\
"Specifying both positional and keyword arguments is not allowed in reshape."
reverse = kwargs.get('reverse', False)
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to make sure kwargs doesn't contain other arguments

@piiswrong piiswrong merged commit 66b2944 into apache:master May 3, 2018
@szha szha deleted the reshape_rev branch May 3, 2018 17:39
marcoabreu added a commit that referenced this pull request May 3, 2018
anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request May 7, 2018
* add reverse option to ndarray inplace reshape

* update check
jinhuang415 pushed a commit to jinhuang415/incubator-mxnet that referenced this pull request May 29, 2018
* add reverse option to ndarray inplace reshape

* update check
rahul003 pushed a commit to rahul003/mxnet that referenced this pull request Jun 4, 2018
* add reverse option to ndarray inplace reshape

* update check
zheng-da pushed a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 28, 2018
* add reverse option to ndarray inplace reshape

* update check
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.

3 participants