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 2021/09/01 08:31:27 UTC

[GitHub] [incubator-mxnet] mozga-intel commented on a change in pull request #20563: [FEATURE] Add oneDNN support for npx.reshape and np.reshape

mozga-intel commented on a change in pull request #20563:
URL: https://github.com/apache/incubator-mxnet/pull/20563#discussion_r699981653



##########
File path: src/operator/nn/mkldnn/mkldnn_convolution.cc
##########
@@ -39,7 +39,7 @@ DMLC_REGISTER_PARAMETER(MKLDNNConvParam);
 bool SupportMKLDNNConv(const ConvolutionParam& params, const NDArray& input) {
   if ((params.kernel.ndim() != 1) && (params.kernel.ndim() != 2) && (params.kernel.ndim() != 3))
     return false;
-  return SupportMKLDNNQuantize(input.dtype()) &&
+  return IsMKLDNNType(input.dtype()) &&
          ((input.shape().ndim() == 3) || (input.shape().ndim() == 4) ||

Review comment:
       BTW: It could be nice to change it, to get less comparison.
   ```suggestion
    input.shape().ndim() >= 3 && input.shape().ndim() <= 5
    ```

##########
File path: src/operator/nn/mkldnn/mkldnn_reshape.cc
##########
@@ -33,6 +33,13 @@
 namespace mxnet {
 namespace op {
 
+bool SupportMKLDNNReshape(const NDArray& input, const NDArray& output) {
+  const int input_ndims  = input.shape().ndim();
+  const int output_ndims = output.shape().ndim();
+  return input_ndims >= 1 && input_ndims <= 6 && output_ndims >= 1 && output_ndims <= 6 &&
+         IsMKLDNNType(input.dtype()) && input.shape().Size() > 0;

Review comment:
       The input is having its size checked at the end of this statement ~ if the Size() function returns the size of the input, then it could be better to place it at the beginning. 

##########
File path: src/operator/nn/mkldnn/mkldnn_convolution.cc
##########
@@ -39,7 +39,7 @@ DMLC_REGISTER_PARAMETER(MKLDNNConvParam);
 bool SupportMKLDNNConv(const ConvolutionParam& params, const NDArray& input) {
   if ((params.kernel.ndim() != 1) && (params.kernel.ndim() != 2) && (params.kernel.ndim() != 3))

Review comment:
       BTW: It could be nice to change it.
   ```suggestion
     if (params.kernel.ndim() > 3)) return false;
   ```
   




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

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org