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/08/14 19:29:52 UTC
[GitHub] [incubator-mxnet] Zha0q1 opened a new pull request #18925: Numpy Dot Large Tensor Fix
Zha0q1 opened a new pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925
This PR fixes numpy dot on large tensors. This fix also requires ILP 64 Blas which CI has not migrated to yet, so for now we are skipping the test.
With this fix numpy dot will no longer error out on > 2**32 dimensions. However there seems to be a precision issue with ILP64 openblas: https://github.com/xianyi/OpenBLAS/issues/2779
So on my remote computer
```
A = np.ones((1, 2**31))
B = np.ones((2**31, 1))
C = np.dot(A, B)
```
Will give 1.93274e+09
```
A = np.ones((2**31, 1), dtype='float64')
B = np.ones((1, 2**31), dtype='float64')
```
Will give the arithmetically correct result 2.14748365e+09
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] access2rohit commented on a change in pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#discussion_r471812842
##########
File path: tests/nightly/test_np_large_array.py
##########
@@ -76,3 +77,11 @@ def test_softmax():
true_output = np.full((SMALL_Y, LARGE_X), (1 / input_data.shape[axis]))
output = npx.softmax(input_data, axis=axis)
assert_almost_equal(output.asnumpy(), true_output, rtol=1e-5, atol=1e-5)
+
+@pytest.mark.skip(reason="CI hasn't switch to ILP64 OpenBLAS yet")
Review comment:
tests for same operator in same file will increase file size. I like the decorator idea but any specific reason to keep both tests in same file ?
@Zha0q1 I will review it in a while
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] access2rohit commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
access2rohit commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-675150160
Can you run the tests using pytest and paste console output here ? (both command and output)
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] Zha0q1 commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-674353771
@sandeep-krishnamurthy
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-674238157
Hey @Zha0q1 , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:
- To trigger all jobs: @mxnet-bot run ci [all]
- To trigger specific jobs: @mxnet-bot run ci [job1, job2]
***
**CI supported jobs**: [centos-gpu, clang, sanity, website, unix-gpu, windows-cpu, centos-cpu, miscellaneous, windows-gpu, unix-cpu, edge]
***
_Note_:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] Zha0q1 commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-675668855
> Rest LGTM ! Can you run the tests manually and paste results here
=================================== test session starts ===================================
platform linux -- Python 3.7.7, pytest-5.4.1, py-1.8.1, pluggy-0.13.1
rootdir: /home/ubuntu/dot_fix, inifile: pytest.ini
plugins: remotedata-0.3.2, openfiles-0.4.0, astropy-header-0.1.2, hypothesis-5.8.3, arraydiff-0.3, doctestplus-0.5.0
collected 1 item
tests/nightly/test_np_large_array.py . [100%]
==================================== 1 passed in 2.75s ===================================
Passes offline.
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] Zha0q1 commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-675727781
@mxnet-bot run ci [unix-cpu]
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] sandeep-krishnamurthy commented on a change in pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
sandeep-krishnamurthy commented on a change in pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#discussion_r471017487
##########
File path: tests/nightly/test_np_large_array.py
##########
@@ -76,3 +77,11 @@ def test_softmax():
true_output = np.full((SMALL_Y, LARGE_X), (1 / input_data.shape[axis]))
output = npx.softmax(input_data, axis=axis)
assert_almost_equal(output.asnumpy(), true_output, rtol=1e-5, atol=1e-5)
+
+@pytest.mark.skip(reason="CI hasn't switch to ILP64 OpenBLAS yet")
Review comment:
Should we move this to nightly test folder?
I was also thinking if we should have a decorator that we can use to mark tests as nightly tests. So we can keep operator tests in same files for maintainability but also not run as UTs for PR checks.
@access2rohit wdyt?
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] access2rohit commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
access2rohit commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-675577003
> This test is skipped for now because we are not yet using ilp64 blas. After we migrate to ILP64 blas/lapck I will unskip this test and check again if it passes.
Can you locally run it and paste the output ? I saw the description but your change should atleast be locally tested.
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on a change in pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#discussion_r471029369
##########
File path: tests/nightly/test_np_large_array.py
##########
@@ -76,3 +77,11 @@ def test_softmax():
true_output = np.full((SMALL_Y, LARGE_X), (1 / input_data.shape[axis]))
output = npx.softmax(input_data, axis=axis)
assert_almost_equal(output.asnumpy(), true_output, rtol=1e-5, atol=1e-5)
+
+@pytest.mark.skip(reason="CI hasn't switch to ILP64 OpenBLAS yet")
Review comment:
I think this is in the nightly folder. Currently we are not using ilp 64 blas on any machine, so this test will fail on nightly as well. Hence I added the skip. Sorry about the confusion.
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] sandeep-krishnamurthy merged pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
sandeep-krishnamurthy merged pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] access2rohit commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
access2rohit commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-675583267
Rest LGTM ! Can you run the tests manually and paste results here
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-674355457
Jenkins CI successfully triggered : [windows-gpu]
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] Zha0q1 commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-675778382
@sandeep-krishnamurthy can you help merge
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-675727815
Jenkins CI successfully triggered : [unix-cpu]
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] szha commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-674355452
@mxnet-bot run ci [windows-gpu]
----------------------------------------------------------------
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
[GitHub] [incubator-mxnet] Zha0q1 commented on pull request #18925: Numpy Dot Large Tensor Fix
Posted by GitBox <gi...@apache.org>.
Zha0q1 commented on pull request #18925:
URL: https://github.com/apache/incubator-mxnet/pull/18925#issuecomment-675151277
> Can you run the tests using pytest and paste console output here ? (both command and output)
This test is skipped for now because we are not yet using ilp64 blas. After we migrate to ILP64 blas/lapck I will unskip this test and check again if it passes.
----------------------------------------------------------------
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