Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Enabling large tensor support for binary broadcast operators #16755

Merged
merged 4 commits into from
Jan 9, 2020

Conversation

ChaiBapchya
Copy link
Contributor

@ChaiBapchya ChaiBapchya commented Nov 7, 2019

Description

In continuation of #16714
Add tests

  • arctan2
  • hypot

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Code is well-documented:
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • tests/nightly/test_large_array.py
  • src/operator/tensor/elemwise_binary_broadcast_op.h

@access2rohit
Copy link
Contributor

LGTM ! Also paste the output for tests run.
Can we also do large vector tests too for arctan2 ? I assume hypt is hypotenuse so maybe that requires 2D matrix

@ChaiBapchya
Copy link
Contributor Author

Build flag

python -c "from mxnet.runtime import feature_list; print(feature_list())"
[✖ CUDA, ✖ CUDNN, ✖ NCCL, ✖ CUDA_RTC, ✖ TENSORRT, ✔ CPU_SSE, ✔ CPU_SSE2, ✔ CPU_SSE3, ✔ CPU_SSE4_1, ✔ CPU_SSE4_2, ✖ CPU_SSE4A, ✔ CPU_AVX, ✖ CPU_AVX2, ✔ OPENMP, ✖ SSE, ✔ F16C, ✖ JEMALLOC, ✔ BLAS_OPEN, ✖ BLAS_ATLAS, ✖ BLAS_MKL, ✖ BLAS_APPLE, ✔ LAPACK, ✔ MKLDNN, ✔ OPENCV, ✖ CAFFE, ✖ PROFILER, ✔ DIST_KVSTORE, ✖ CXX14, ✔ INT64_TENSOR_SIZE, ✔ SIGNAL_HANDLER, ✖ DEBUG, ✖ TVM_OP]

Output of test run

ubuntu@ip-172-31-27-86:~/workspace/incubator-mxnet$ nosetests tests/nightly/test_large_array.py:test_binary_broadcast
.
----------------------------------------------------------------------
Ran 1 test in 39.432s

OK

@ChaiBapchya
Copy link
Contributor Author

LGTM ! Also paste the output for tests run.
Can we also do large vector tests too for arctan2 ? I assume hypt is hypotenuse so maybe that requires 2D matrix

hypot can work for vector too (it takes 2 values as input which basically act as 2 sides of the triangle) - shape of the 2 values has to be the same / broadcastable

@ChaiBapchya
Copy link
Contributor Author

Vector test

nosetests tests/nightly/test_large_vector.py:test_binary_broadcast
.
----------------------------------------------------------------------
Ran 1 test in 20.125s

OK

@access2rohit
Copy link
Contributor

Vector test

nosetests tests/nightly/test_large_vector.py:test_binary_broadcast
.
----------------------------------------------------------------------
Ran 1 test in 20.125s

OK

Did also you run the whole suite for tensor and vector ?

@ChaiBapchya
Copy link
Contributor Author

ChaiBapchya commented Dec 31, 2019

python -m nose tests/nightly/test_large_array.py:test_tvm_add
.
----------------------------------------------------------------------
Ran 1 test in 10.660s

OK

However, tvm_add test fails when individual dimension size > 2**32.
for eg in case of test_large_vector.py SHAPE = (4300000000) test fails.

Verified with @yzhliu that change needs to be done on the tvm side. It's a non-trivial change. and thus the test_tvm_add will be added in subsequent separate PR.

@ChaiBapchya
Copy link
Contributor Author

Vector test

nosetests tests/nightly/test_large_vector.py:test_binary_broadcast
.
----------------------------------------------------------------------
Ran 1 test in 20.125s

OK

Did also you run the whole suite for tensor and vector ?

Yes. As noted previously, on 480GB CPU test_large_array needs to be run twice (first half tests followed by second half tests approx) due to memory free-ing issue.

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@apeforest apeforest merged commit 6ba9aad into apache:master Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants