[GitHub] [incubator-mxnet] sxjscience commented on a change in pull request #15560: Add fp16 support for topk

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

sxjscience commented on a change in pull request #15560: Add fp16 support for topk
URL: https://github.com/apache/incubator-mxnet/pull/15560#discussion_r304689263
 
 

 ##########
 File path: src/operator/tensor/ordering_op-inl.h
 ##########
 @@ -420,8 +420,6 @@ void TopKImpl(const RunContext &ctx,
     mxnet::op::SortByKeyWorkspaceSize<int, int, xpu>(src.Size()));
   temp_size = std::max(temp_size,
     mxnet::op::SortByKeyWorkspaceSize<int, DType, xpu>(src.Size()));
-  temp_size = std::max(temp_size,
 
 Review comment:
   I think this line is to make sure that the code will be correct no matter how we change the `SortByKeyWorkspaceSize` function. For example, if we later change our `SortByKeyWorkspaceSize` as something like:
   ```
   mxnet::op::SortByKeyWorkspaceSize<int, DType, xpu>() --> Return 0
   mxnet::op::SortByKeyWorkspaceSize<DType, int, xpu>() --> Return 1000000
   ```
   The current revision will raise an error because the `temp_size` is not correctly updated. I suggest keeping this line to avoid issues in the future.

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