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/07/30 07:28:48 UTC
[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #18816: Add Large Dim Checks for linalg Operators
ChaiBapchya commented on a change in pull request #18816:
URL: https://github.com/apache/incubator-mxnet/pull/18816#discussion_r462660405
##########
File path: tests/nightly/test_large_array.py
##########
@@ -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))
Review 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
##########
File path: tests/nightly/test_large_array.py
##########
@@ -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))
Review comment:
Name of the file can be made better.
Basically the idea was to have 2 separate files
1. test_large_array.py [more like large size]
testing input whose individual dimensions are less than `2**32` but size of the input is > `2**32`
2. test_large_vector.py [more like large "shape"]
testing input whose atleast 1 individual dimensions is > `2**32`
##########
File path: tests/nightly/test_large_array.py
##########
@@ -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))
Review comment:
Name of the file can be made better.
Basically the idea was to have 2 separate files
1. test_large_array.py [more like test_large_**size**.py]
testing input whose individual dimensions are less than `2**32` but size of the input is > `2**32`
2. test_large_vector.py [more like test_large_**shape**.py]
testing input whose atleast 1 individual dimensions is > `2**32`
##########
File path: tests/nightly/test_large_array.py
##########
@@ -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))
Review comment:
From consistency standpoint, I'd
1. put these testss in test_large_vector.py file.
2. Rename that file to [whatever sounds right I just gave a suggestion above]
3. 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".
##########
File path: tests/nightly/test_large_array.py
##########
@@ -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))
Review comment:
From consistency standpoint, I'd
1. put these tests in test_large_vector.py file.
2. Rename that file to [whatever sounds right I just gave a suggestion above]
3. 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".
##########
File path: tests/nightly/test_large_array.py
##########
@@ -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))
Review comment:
Yes I know they aren't "vectors" and hence recommended "renaming the file"
##########
File path: tests/nightly/test_large_array.py
##########
@@ -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))
Review comment:
If we keep these tests in this file, it defeats the purpose of few tests in test_large_vector.py
https://github.com/apache/incubator-mxnet/blob/6bbd53107aa16fc41e8d462cf5dc46fb70d592df/tests/nightly/test_large_vector.py#L99-L126
----------------------------------------------------------------
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