-
Notifications
You must be signed in to change notification settings - Fork 6.8k
add reverse option to ndarray inplace reshape #10767
Conversation
I think this is over complicated. besides the symbolic version doesn't support it |
It's used a lot in attention, and the in-place option can make a huge difference in saving memory. |
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, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
ping @piiswrong |
python/mxnet/ndarray/ndarray.py
Outdated
else: | ||
assert not kwargs,\ | ||
"Specifying both positional and keyword arguments is not allowed in reshape." | ||
reverse = kwargs.get('reverse', False) |
There was a problem hiding this comment.
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
This reverts commit 66b2944.
* add reverse option to ndarray inplace reshape * update check
* add reverse option to ndarray inplace reshape * update check
* add reverse option to ndarray inplace reshape * update check
* add reverse option to ndarray inplace reshape * update check
Description
add reverse option to ndarray inplace reshape
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes