[GitHub] [incubator-mxnet] asmushetzel commented on issue #13862: [1.4.0] unravel_index no longer works with magic '-1' in shape parameter as in 1.3.1

[GitBox <gi...@apache.org>]

asmushetzel commented on issue #13862: [1.4.0] unravel_index no longer works with magic '-1' in shape parameter as in 1.3.1
URL: https://github.com/apache/incubator-mxnet/issues/13862#issuecomment-470110394
 
 
   Pretty easy to diagnose. 
   
   The type for mshadow::index_t got changed from "unsigned" to "int64_t" between both versions. This type is used to encode dimensions of shapes internally. 
   In 1.3.1, a "-1"  therefore was interpreted as unsigned(-1) which is basically max_int. In 1.4.0 and later it is interpreted as "-1" and that changed the behaviour of the operator. 
   
   It is debatable whether we ever wanted to explicitly allow the use of "-1" in the shape argument of the operator. At least I as the original author had this not in mind and also the documentation does not say anything about magic numbers. In fact, the only case where the operator did work (and could ever work) with magic numbers is when "-1" is specified as first coordinate.  Nothing else is possible. 
   Two solutions:
   - We document that it is o.k. to use "-1" as first shape coordinate in ravel/unravel_index. And enhance the operator to support this. That does not require any change for "ravel" and a simple change in the code shown below. 
   - We leave it as is (the code simply does not support this). A potential workaround for customers would be to specify a huge number except "-1" for the first dimension which has essentially the same effect. 
   
   ```
   struct unravel_index {
     template<typename DType>
     MSHADOW_XINLINE static void Map(int i, index_t N, index_t ndim, index_t *shape,
                                     DType *unravelled, DType *ravelled) {
       index_t idx(ravelled[i]);
       #pragma unroll
       for (int j = ndim; j--; ) {
         index_t tmp = idx / shape[j];
         unravelled[i+j*N] = idx - tmp*shape[j];
         idx = tmp;
       }
     }
   };
   ```
   
   
   
   
   
   
   
   

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


With regards,
Apache Git Services