Optimize ERF with MKL math function #5
Conversation
| @with_seed() | ||
| def test_math(): | ||
| ops = ['log', 'erf', 'square'] | ||
| check_value= True |
| ops = ['log', 'erf', 'square'] | ||
| check_value= True | ||
| lshape = 1000 | ||
| rshapes = [1, 10, 100, 1000, 10000] |
There was a problem hiding this comment.
Is there any reason why you picked 5 shapes?
There was a problem hiding this comment.
The test case is initialy to evaluate the scalability of mkl vml functions on different size of tensors, since it is not proper to track the time cost on calculation, I will modify the shape to 1d, 2d, 3d tensors
| def math_square(shape, dtype, check_value): | ||
| np_x = np.random.rand(shape[0], shape[1]) | ||
| x = mx.nd.array(np_x, dtype=dtype) | ||
| mx.nd.waitall() |
There was a problem hiding this comment.
Why do we need to frequently add waitall and wait_to_read in tests?
There was a problem hiding this comment.
The test case is initially to calcuatee the time cost of the calculations on different size tensors, wwwill remove the waitall, wait_to_read fron the test case as the test case targting the correctness of the calculation.
| @@ -35,9 +35,10 @@ | |||
| #include "../mxnet_op.h" | |||
| #include "../elemwise_op_common.h" | |||
| #include "../../ndarray/ndarray_function.h" | |||
|
|
|||
| #if MSHADOW_USE_MKL == 1 | |||
There was a problem hiding this comment.
I have not set USE_MKL before. Just curious: is blas=MKL also tested in mxnet CI?
There was a problem hiding this comment.
MSHADOW_USE_MKL is widely used in mshadow and mxnet to indicate MKL is used as BLAS library. Yes, USE_BLAS=mkl is built and tested in CI:
https://github.com/apache/incubator-mxnet/blob/master/ci/docker/runtime_functions.sh#L375
https://github.com/apache/incubator-mxnet/blob/master/ci/docker/runtime_functions.sh#L553
| * With this macro means mxnet compile with MKL to accelerate math function with mkl. | ||
| * Will Register FCompute with UnaryOp::MKL_Compute() to compelet the math function. | ||
| */ | ||
| #define MXNET_MKL_OPERATOR_REGISTER_UNARY(__name$) \ |
There was a problem hiding this comment.
How is this different from MXNET_OPERATOR_REGISTER_UNARY?
There was a problem hiding this comment.
The implementation of these two macros are same, duplicates code removed.
|
|
||
| )code" ADD_FILELINE) | ||
| .set_attr<nnvm::FGradient>("FGradient", ElemwiseGradUseIn{"_backward_square"}); | ||
| #else |
There was a problem hiding this comment.
Hmmm this does not scale to 100 ops. We need to think about better ways.
There was a problem hiding this comment.
Yes. Need revisit the design to make it scalable for both operator registration and the kernel launching everywhere in mxnet.
Fix review comments
Description
(Brief description on what this PR is about)
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments