[GitHub] stephenrawls commented on issue #14053: in-place reshape ops

[GitBox]

stephenrawls commented on issue #14053: in-place reshape ops
URL: https://github.com/apache/incubator-mxnet/pull/14053#issuecomment-460090968
 
 
   @szha -- Thanks for the detailed explanation, that helps a lot.
   
   So just for clarity, I see that in the operator definition for expand_dims, 
it sets the FInPlaceIdentity attribute to true:
   
https://github.com/apache/incubator-mxnet/blob/45d1a1e6ff9c58cfc75f72651c1bf671ac7f1885/src/operator/tensor/matrix_op.cc#L400-L419
   
   I didn't see anything in the plan_memory.cc file that depended on autograd / 
training.
   
   So should I take it that the code is smart enough to not allocate new memory 
for the expand_dims operator in symbolic mode, i.e. not to make a copy, even 
during training when using autograd?
   
   If so, then I think I understand it, and I see what you mean about only 
needing to fix this for the Python ndarray use case.
   
   Thanks again!
   Stephen


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] stephenrawls commented on issue #14053: in-place reshape ops

[GitBox]

stephenrawls commented on issue #14053: in-place reshape ops
URL: https://github.com/apache/incubator-mxnet/pull/14053#issuecomment-460005020
 
 
   @szha -- Thanks for putting in this patch!
   
   I have a couple questions about the operator `expand_dims`, which I 
understand you are actually changing the python code to *not* use that 
operator, but I think still has relevance.
   
   (1) I originally discovered this because I was using the C API to call 
MXImperativeInvoke() using the expand_dims operator, and I noticed it was 
causing a copy. This fix only effects the Python version when operating on 
ndarrays, not users of the expand_dims operator.
   
   I see that there is a path in the operator that checks if it is an in-place 
operation. Presumably it uses that path if I pass an output array that is the 
same NDArrayHandle as the input array? But what if I still need the original 
input array handle, and I want to create a new output array handle with the 
expanded dim but still not make a copy?
   
   (2) In the issue I created you commented that: "For symbol (and thus the 
hybridized version), since in-place identity is possible it should not matter".
   
   Can you talk a little more about that? I assume you mean that in this case:
   ```
   x_expanded = x.expand_dims(1)
   y = x_expanded + foo
   ```
   The engine can figure out that x is not needed again, and can thus turn the 
expand_dims(1) into an in-place operation that doesn't make a copy?
   
   I'm not very familiar with how this part of the code works, so what happens 
if you had code that looked like this?
   ```
   x_expanded = x.expand_dims(1)
   y = x_expanded + foo
   z = 2 * x
   ```
   i.e. the code still makes a reference to the original x, and thus presumably 
the engine can't decide to use the in-place version of expand_dims in that 
case, right? So I guess my question is -- Does the ability for the Syblolic / 
hybridized engine to elide the copy depend on the code not referencing the 
un-expanded version of the array after calling expand_dims()? If so, it seems 
like there will still be some use cases where an unexpected copy is happening.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services