You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/01/14 21:17:23 UTC

[GitHub] [incubator-mxnet] access2rohit opened a new pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

access2rohit opened a new pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309
 
 
   ## Description ##
   adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs:
   MXNDArrayGetShape64
   MXSymbolInferShape64
   MXSymbolInferShapePartial64
   
   ## Checklist ##
   ### Essentials ###
   Please feel free to remove inapplicable items for your PR.
   - [x] Changes are complete (i.e. I finished coding on this PR)
   - [x] All changes have test coverage:
   - Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
   - Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
   - Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
   - [x] Code is well-documented: 
   - For user-facing API changes, API doc string has been updated. 
   - For new C++ functions in header files, their functionalities and arguments are documented. 
   - For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
   - [x] To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
   

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366607671
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1840,6 +1926,32 @@ MXNET_DLL int MXSymbolInferShapePartialEx(SymbolHandle sym,
                                           const int ***aux_shape_data,
                                           int *complete);
 
+/*!
+ * \brief partially infer shape of unknown input shapes given the known one.
+ *
+ *  Return partially inferred results if not all shapes could be inferred.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ *
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
 
 Review comment:
   ```suggestion
    * \param num_args number of input arguments.
   ```

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366606053
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1840,6 +1926,32 @@ MXNET_DLL int MXSymbolInferShapePartialEx(SymbolHandle sym,
                                           const int ***aux_shape_data,
                                           int *complete);
 
+/*!
+ * \brief partially infer shape of unknown input shapes given the known one.
+ *
+ *  Return partially inferred results if not all shapes could be inferred.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ *
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
+ * \param keys the key of keyword args (optional)
+ * \param arg_ind_ptr the head pointer of the rows in CSR
+ * \param arg_shape_data the content of the CSR
+ * \param in_shape_size sizeof the returning array of in_shapes
+ * \param in_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param in_shape_data returning array of pointers to head of the input shape.
+ * \param out_shape_size sizeof the returning array of out_shapes
+ * \param out_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param out_shape_data returning array of pointers to head of the input shape.
+ * \param aux_shape_size sizeof the returning array of aux_shapes
 
 Review comment:
   ```suggestion
    * \param aux_shape_size size of the returning array of aux_shapes
   ```

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r367037006
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1840,6 +1926,32 @@ MXNET_DLL int MXSymbolInferShapePartialEx(SymbolHandle sym,
                                           const int ***aux_shape_data,
                                           int *complete);
 
+/*!
+ * \brief partially infer shape of unknown input shapes given the known one.
+ *
+ *  Return partially inferred results if not all shapes could be inferred.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ *
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
+ * \param keys the key of keyword args (optional)
+ * \param arg_ind_ptr the head pointer of the rows in CSR
+ * \param arg_shape_data the content of the CSR
+ * \param in_shape_size sizeof the returning array of in_shapes
+ * \param in_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param in_shape_data returning array of pointers to head of the input shape.
+ * \param out_shape_size sizeof the returning array of out_shapes
+ * \param out_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param out_shape_data returning array of pointers to head of the input shape.
+ * \param aux_shape_size sizeof the returning array of aux_shapes
 
 Review comment:
   addressed all comments. sizeof can also be also thought of as a function, which I think is fine. So, decided to keep it that way. Let me know if its important to make it "size of". I don't have any strong opinion on keeping it "sizeof"

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366605886
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1840,6 +1926,32 @@ MXNET_DLL int MXSymbolInferShapePartialEx(SymbolHandle sym,
                                           const int ***aux_shape_data,
                                           int *complete);
 
+/*!
+ * \brief partially infer shape of unknown input shapes given the known one.
+ *
+ *  Return partially inferred results if not all shapes could be inferred.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ *
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
+ * \param keys the key of keyword args (optional)
+ * \param arg_ind_ptr the head pointer of the rows in CSR
+ * \param arg_shape_data the content of the CSR
+ * \param in_shape_size sizeof the returning array of in_shapes
+ * \param in_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param in_shape_data returning array of pointers to head of the input shape.
+ * \param out_shape_size sizeof the returning array of out_shapes
+ * \param out_shape_ndim returning array of shape dimensions of eachs input shape.
 
 Review comment:
   
   ```suggestion
    * \param out_shape_ndim returning array of shape dimensions of eachs output shape.
   ```

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366606283
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1727,6 +1804,29 @@ MXNET_DLL int MXSymbolInferShapeEx(SymbolHandle sym,
                                    const int ***aux_shape_data,
                                    int *complete);
 
+/*!
+ * \brief infer shape of unknown input shapes given the known one.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
+ * \param keys the key of keyword args (optional)
+ * \param arg_ind_ptr the head pointer of the rows in CSR
+ * \param arg_shape_data the content of the CSR
+ * \param in_shape_size sizeof the returning array of in_shapes
+ * \param in_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param in_shape_data returning array of pointers to head of the input shape.
+ * \param out_shape_size sizeof the returning array of out_shapes
+ * \param out_shape_ndim returning array of shape dimensions of eachs input shape.
 
 Review comment:
   ```suggestion
    * \param out_shape_ndim returning array of shape dimensions of each output shape.
   ```

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366609591
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -585,6 +585,8 @@ MXNET_DLL int MXNDArrayCreate(const uint32_t *shape,
 
 /*!
  * \brief create a NDArray with specified shape and data type
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=0 (by default)
 
 Review comment:
   There already is a functionality to throw error messages when usage is of large tensor and mxnet is not built with USE_INT64_TENSOR_SIZE=1. This is a documentation for devs to help them decide which one to choose.

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] access2rohit commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
access2rohit commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#issuecomment-574378172
 
 
   @mxnet-label-bot add [pr-awaiting-review]

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366606476
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1727,6 +1804,29 @@ MXNET_DLL int MXSymbolInferShapeEx(SymbolHandle sym,
                                    const int ***aux_shape_data,
                                    int *complete);
 
+/*!
+ * \brief infer shape of unknown input shapes given the known one.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
 
 Review comment:
   ```suggestion
    * \param num_args number of input arguments.
   ```

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] apeforest commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
apeforest commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#issuecomment-575969876
 
 
   I think the *64APIs were introduce in 1.6. @access2rohit Please confirm. Thanks.

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r367037006
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1840,6 +1926,32 @@ MXNET_DLL int MXSymbolInferShapePartialEx(SymbolHandle sym,
                                           const int ***aux_shape_data,
                                           int *complete);
 
+/*!
+ * \brief partially infer shape of unknown input shapes given the known one.
+ *
+ *  Return partially inferred results if not all shapes could be inferred.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ *
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
+ * \param keys the key of keyword args (optional)
+ * \param arg_ind_ptr the head pointer of the rows in CSR
+ * \param arg_shape_data the content of the CSR
+ * \param in_shape_size sizeof the returning array of in_shapes
+ * \param in_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param in_shape_data returning array of pointers to head of the input shape.
+ * \param out_shape_size sizeof the returning array of out_shapes
+ * \param out_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param out_shape_data returning array of pointers to head of the input shape.
+ * \param aux_shape_size sizeof the returning array of aux_shapes
 
 Review comment:
   addressed all comments. sizeof can also be also thought of as a function, which I think is fine. So, decided to keep it that way.

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] access2rohit commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
access2rohit commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#issuecomment-574378284
 
 
   @apeforest @ChaiBapchya PR ready for review

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r367037006
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1840,6 +1926,32 @@ MXNET_DLL int MXSymbolInferShapePartialEx(SymbolHandle sym,
                                           const int ***aux_shape_data,
                                           int *complete);
 
+/*!
+ * \brief partially infer shape of unknown input shapes given the known one.
+ *
+ *  Return partially inferred results if not all shapes could be inferred.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ *
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
+ * \param keys the key of keyword args (optional)
+ * \param arg_ind_ptr the head pointer of the rows in CSR
+ * \param arg_shape_data the content of the CSR
+ * \param in_shape_size sizeof the returning array of in_shapes
+ * \param in_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param in_shape_data returning array of pointers to head of the input shape.
+ * \param out_shape_size sizeof the returning array of out_shapes
+ * \param out_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param out_shape_data returning array of pointers to head of the input shape.
+ * \param aux_shape_size sizeof the returning array of aux_shapes
 
 Review comment:
   addressed all comments. sizeof can also be thought of as a function either is fine so decided to keep it that way.

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366608184
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1784,28 +1884,14 @@ MXNET_DLL int MXSymbolInferShapePartial(SymbolHandle sym,
                                         const uint32_t ***aux_shape_data,
                                         int *complete);
 
-MXNET_DLL int MXSymbolInferShapePartial64(SymbolHandle sym,
-                                          uint32_t num_args,
-                                          const char** keys,
-                                          const int64_t *arg_ind_ptr,
-                                          const int64_t *arg_shape_data,
-                                          size_t *in_shape_size,
-                                          const int **in_shape_ndim,
-                                          const int64_t ***in_shape_data,
-                                          size_t *out_shape_size,
-                                          const int **out_shape_ndim,
-                                          const int64_t ***out_shape_data,
-                                          size_t *aux_shape_size,
-                                          const int **aux_shape_ndim,
-                                          const int64_t ***aux_shape_data,
-                                          int *complete);
-
 /*!
  * \brief partially infer shape of unknown input shapes given the known one.
  *
  *  Return partially inferred results if not all shapes could be inferred.
  *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
  *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=0 (by default)
  *
  * \param sym symbol handle
  * \param num_args numbe of input arguments.
 
 Review comment:
   ```suggestion
    * \param num_args number of input arguments.
   ```

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] szha commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
szha commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#issuecomment-575965161
 
 
   @access2rohit @apeforest could you help clarify which version the *64 APIs were first introduced? If they appeared in the past versions before, then this change cannot be merged into 1.7.x (#16864)

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r367037006
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1840,6 +1926,32 @@ MXNET_DLL int MXSymbolInferShapePartialEx(SymbolHandle sym,
                                           const int ***aux_shape_data,
                                           int *complete);
 
+/*!
+ * \brief partially infer shape of unknown input shapes given the known one.
+ *
+ *  Return partially inferred results if not all shapes could be inferred.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ *
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
+ * \param keys the key of keyword args (optional)
+ * \param arg_ind_ptr the head pointer of the rows in CSR
+ * \param arg_shape_data the content of the CSR
+ * \param in_shape_size sizeof the returning array of in_shapes
+ * \param in_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param in_shape_data returning array of pointers to head of the input shape.
+ * \param out_shape_size sizeof the returning array of out_shapes
+ * \param out_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param out_shape_data returning array of pointers to head of the input shape.
+ * \param aux_shape_size sizeof the returning array of aux_shapes
 
 Review comment:
   addressed all comments. sizeof can also be also thought of as a function, which I think is fine. So, decided to keep it that way. Let me know if it important to make it "size of". I don't have any strong opinion on keeping it "sizeof"

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
apeforest commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366588180
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -585,6 +585,8 @@ MXNET_DLL int MXNDArrayCreate(const uint32_t *shape,
 
 /*!
  * \brief create a NDArray with specified shape and data type
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=0 (by default)
 
 Review comment:
   I think instead of adding the comment, we should just error out from the API when the incorrect flags are used with API.

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] apeforest merged pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
apeforest merged pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309
 
 
   

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366608014
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1673,27 +1765,12 @@ MXNET_DLL int MXSymbolInferShape(SymbolHandle sym,
                                  const uint32_t ***aux_shape_data,
                                  int *complete);
 
-MXNET_DLL int MXSymbolInferShape64(SymbolHandle sym,
-                                   uint32_t num_args,
-                                   const char** keys,
-                                   const int64_t *arg_ind_ptr,
-                                   const int64_t *arg_shape_data,
-                                   size_t *in_shape_size,
-                                   const int **in_shape_ndim,
-                                   const int64_t ***in_shape_data,
-                                   size_t *out_shape_size,
-                                   const int **out_shape_ndim,
-                                   const int64_t ***out_shape_data,
-                                   size_t *aux_shape_size,
-                                   const int **aux_shape_ndim,
-                                   const int64_t ***aux_shape_data,
-                                   int *complete);
-
 /*!
  * \brief infer shape of unknown input shapes given the known one.
  *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
  *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
- *
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=0 (by default)
  * \param sym symbol handle
  * \param num_args numbe of input arguments.
 
 Review comment:
   ```suggestion
    * \param num_args number of input arguments.
   ```

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366606351
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1727,6 +1804,29 @@ MXNET_DLL int MXSymbolInferShapeEx(SymbolHandle sym,
                                    const int ***aux_shape_data,
                                    int *complete);
 
+/*!
+ * \brief infer shape of unknown input shapes given the known one.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
+ * \param keys the key of keyword args (optional)
+ * \param arg_ind_ptr the head pointer of the rows in CSR
+ * \param arg_shape_data the content of the CSR
+ * \param in_shape_size sizeof the returning array of in_shapes
+ * \param in_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param in_shape_data returning array of pointers to head of the input shape.
+ * \param out_shape_size sizeof the returning array of out_shapes
+ * \param out_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param out_shape_data returning array of pointers to head of the input shape.
 
 Review comment:
   ```suggestion
    * \param out_shape_data returning array of pointers to head of the output shape.
   ```

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] access2rohit commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
access2rohit commented on issue #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#issuecomment-577425103
 
 
   > I think the *64APIs were introduce in 1.6. @access2rohit Please confirm. Thanks.
   
   Correct ! 

----------------------------------------------------------------
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

[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs

Posted by GitBox <gi...@apache.org>.
ChaiBapchya commented on a change in pull request #17309: adding docs for 64bit C APIs of large tensor and removing not required int64 C APIs 
URL: https://github.com/apache/incubator-mxnet/pull/17309#discussion_r366605962
 
 

 ##########
 File path: include/mxnet/c_api.h
 ##########
 @@ -1840,6 +1926,32 @@ MXNET_DLL int MXSymbolInferShapePartialEx(SymbolHandle sym,
                                           const int ***aux_shape_data,
                                           int *complete);
 
+/*!
+ * \brief partially infer shape of unknown input shapes given the known one.
+ *
+ *  Return partially inferred results if not all shapes could be inferred.
+ *  The shapes are packed into a CSR matrix represented by arg_ind_ptr and arg_shape_data
+ *  The call will be treated as a kwargs call if key != nullptr or num_args==0, otherwise it is positional.
+ *  This api is available when MXNet is built with flag
+ *  USE_INT64_TENSOR_SIZE=1 (not default) i.e. Large Tensor Support
+ *
+ * \param sym symbol handle
+ * \param num_args numbe of input arguments.
+ * \param keys the key of keyword args (optional)
+ * \param arg_ind_ptr the head pointer of the rows in CSR
+ * \param arg_shape_data the content of the CSR
+ * \param in_shape_size sizeof the returning array of in_shapes
+ * \param in_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param in_shape_data returning array of pointers to head of the input shape.
+ * \param out_shape_size sizeof the returning array of out_shapes
+ * \param out_shape_ndim returning array of shape dimensions of eachs input shape.
+ * \param out_shape_data returning array of pointers to head of the input shape.
 
 Review comment:
   ```suggestion
    * \param out_shape_data returning array of pointers to head of the output shape.
   ```

----------------------------------------------------------------
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