Add Large Dim Checks for linalg Operators #18816
Conversation
|
Hey @Zha0q1 , Thanks for submitting the PR
CI supported jobs: [unix-gpu, centos-cpu, windows-cpu, unix-cpu, sanity, centos-gpu, website, edge, clang, windows-gpu, miscellaneous] Note: |
Zha0q1
changed the title
tests/nightly/test_large_array.py
Outdated
| @@ -1350,6 +1351,50 @@ def run_trsm(inp): | |||
| check_batch_trsm() | |||
|
|
|||
|
|
|||
| def test_linalg_large_dim(): | |||
| def check_gemm(): | |||
| A = nd.ones(shape=(1, INT32_MAX + 1, 1)) | |||
There was a problem hiding this comment.
this should go to test_large_vector since the input contains 1 dimension which has large while rest dimensions are small. @access2rohit plz confirm
There was a problem hiding this comment.
Name of the file can be made better.
Basically the idea was to have 2 separate files
-
test_large_array.py [more like test_large_size.py]
testing input whose individual dimensions are less than2**32but size of the input is >2**32 -
test_large_vector.py [more like test_large_shape.py]
testing input whose atleast 1 individual dimensions is >2**32
There was a problem hiding this comment.
Maybe we should make more explicit comments on what those test do? I can do so in my next commit. I still think the two cases both fall into the same category which is testing with tensors of large dimensions.
There was a problem hiding this comment.
In comments I would say like 1. those tests are for overflowing total size 2. those other tests are for overflowing index calculation i.e. row dim, col dim, etc
There was a problem hiding this comment.
From consistency standpoint, I'd
- put these tests in test_large_vector.py file.
- Rename that file to [whatever sounds right I just gave a suggestion above]
- add a comment in that file.
All dimensions in test_large_array.py are less than INT32_MAX
Large dimension [>2**32] was introduced in test_large_vector.py for the same reason.
So to keep the testing approach consistent I'd do that.
Even if both files play with "Large tensors" one does it for large "size" other specifically for large "shape".
There was a problem hiding this comment.
well one thing to note is that they are not vectors per se. The inputs are all 3D whereas in test_large_vector they are all 1D. The dim checks happen on both row and col dim so you can see I used both (1, 1, x) and (1, x, 1).
I will add more comments in next commit
There was a problem hiding this comment.
Yes I know they aren't "vectors" and hence recommended "renaming the file"
There was a problem hiding this comment.
If we keep these tests in this file, it defeats the purpose of few tests in test_large_vector.py
There was a problem hiding this comment.
@Zha0q1 the vector file generally houses tests for operators with a single dimension that exceeds 2^32 range. Please address what is suggested by @ChaiBapchya and move it to vector tests. Feel free to rename the file to test_large_dimensions.py
There was a problem hiding this comment.
Yeah that would make sense. I have moved the tests to test_large_vector.py, which I kept the original name to avoid naming discrepancy with master. Also comments were added to the tests
Thanks! |
…tor-mxnet into add_linalg_large_dim_check
Add Large Dim Checks for linalg Operators. Although external blas libraries support large tensors (>2^32 sized), large dimensions (>= 2^31) will trigger an openblas int overflow error under current configuration. This PR adds checks to exit those use cases properly.
Done:
TODO:
All done. The rest of operators take square matrices as inputs, which cannot possibly have large dimensions (>= 2^31) because of memory constrains. For example a 2^32 * 2*31 float matrix will take up 2^24 TB of memory!
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments