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/18 00:04:46 UTC
[GitHub] [incubator-mxnet] josephevans opened a new pull request #18752: [WIP] Fix linalg_potri operator for large tensor.
josephevans opened a new pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752
## Description ##
Fix linalg_potri operator when using large tensor size.
Unit tests to come.
----------------------------------------------------------------
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 #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#discussion_r460149174
##########
File path: tests/nightly/test_large_array.py
##########
@@ -1167,6 +1168,32 @@ def check_correctness(mxnet_op, numpy_op, atol=1e-3):
check_gather()
check_binary_broadcast()
+def test_linalg():
+ def check_potrf():
+ # creating an identity matrix input
+ A = nd.zeros((LARGE_SQ_X, LARGE_SQ_X))
+ for i in range(LARGE_SQ_X):
+ A[i,i] = 1
+
+ out = nd.linalg.potrf(A)
+ # output should be an identity matrix
+ for i in range(LARGE_SQ_X):
+ assert out[i,i] == 1
+
+ def check_potri():
+ # creating an identity matrix input
+ A = nd.zeros((LARGE_SQ_X, LARGE_SQ_X))
+ for i in range(LARGE_SQ_X):
+ A[i,i] = 1
+
+ out = nd.linalg.potri(A)
+ # output should be an identity matrix
+ for i in range(LARGE_SQ_X):
Review comment:
same
----------------------------------------------------------------
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 #18752: [WIP] Fix linalg_potri operator for large tensor.
Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#discussion_r456721100
##########
File path: src/operator/tensor/la_op-inl.h
##########
@@ -36,7 +36,7 @@ using namespace mshadow;
// Copies lower/upper triangular part to upper/lower, i.e. to the opposite side.
struct CopyTriangularToOppositeSide {
template<typename DType>
- MSHADOW_XINLINE static void Map(int i, int matrix_size, int stride, DType* data, bool to_lower) {
+ MSHADOW_XINLINE static void Map(int i, size_t matrix_size, int stride, DType* data, bool to_lower) {
// Below computation works even when we are dealing with a batch of matrices.
const int row((i % matrix_size) / stride), col(i % stride);
Review comment:
row, col int-> index_t
----------------------------------------------------------------
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] josephevans commented on pull request #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
josephevans commented on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-663250225
@mxnet-bot run ci [unix-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] access2rohit commented on pull request #18752: [WIP] Fix linalg_potri operator for large tensor.
Posted by GitBox <gi...@apache.org>.
access2rohit commented on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-660388094
@josephevans Thanks for contribution to Large Tensor Support project. Also paste the output of your test runs as a comment or in the description in the PR.
----------------------------------------------------------------
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 #18752: [WIP] Fix linalg_potri operator for large tensor.
Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#discussion_r456721056
##########
File path: src/operator/tensor/la_op-inl.h
##########
@@ -36,7 +36,7 @@ using namespace mshadow;
// Copies lower/upper triangular part to upper/lower, i.e. to the opposite side.
struct CopyTriangularToOppositeSide {
template<typename DType>
- MSHADOW_XINLINE static void Map(int i, int matrix_size, int stride, DType* data, bool to_lower) {
+ MSHADOW_XINLINE static void Map(int i, size_t matrix_size, int stride, DType* data, bool to_lower) {
Review comment:
i, stride also int->index_t
----------------------------------------------------------------
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 #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#discussion_r460149094
##########
File path: tests/nightly/test_large_array.py
##########
@@ -1167,6 +1168,32 @@ def check_correctness(mxnet_op, numpy_op, atol=1e-3):
check_gather()
check_binary_broadcast()
+def test_linalg():
+ def check_potrf():
+ # creating an identity matrix input
+ A = nd.zeros((LARGE_SQ_X, LARGE_SQ_X))
+ for i in range(LARGE_SQ_X):
+ A[i,i] = 1
+
+ out = nd.linalg.potrf(A)
+ # output should be an identity matrix
+ for i in range(LARGE_SQ_X):
Review comment:
test for only 1 element. Its fine!
----------------------------------------------------------------
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 #18752: [WIP] Fix linalg_potri operator for large tensor.
Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#discussion_r456721056
##########
File path: src/operator/tensor/la_op-inl.h
##########
@@ -36,7 +36,7 @@ using namespace mshadow;
// Copies lower/upper triangular part to upper/lower, i.e. to the opposite side.
struct CopyTriangularToOppositeSide {
template<typename DType>
- MSHADOW_XINLINE static void Map(int i, int matrix_size, int stride, DType* data, bool to_lower) {
+ MSHADOW_XINLINE static void Map(int i, size_t matrix_size, int stride, DType* data, bool to_lower) {
Review comment:
stride also int->index_t
----------------------------------------------------------------
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 #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-663250259
Jenkins CI successfully triggered : [unix-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] mxnet-bot commented on pull request #18752: [WIP] Fix linalg_potri operator for large tensor.
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-660387316
Hey @josephevans , 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**: [unix-gpu, edge, unix-cpu, centos-gpu, windows-gpu, centos-cpu, website, windows-cpu, miscellaneous, sanity, clang]
***
_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] access2rohit commented on a change in pull request #18752: [WIP] Fix linalg_potri operator for large tensor.
Posted by GitBox <gi...@apache.org>.
access2rohit commented on a change in pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#discussion_r456721100
##########
File path: src/operator/tensor/la_op-inl.h
##########
@@ -36,7 +36,7 @@ using namespace mshadow;
// Copies lower/upper triangular part to upper/lower, i.e. to the opposite side.
struct CopyTriangularToOppositeSide {
template<typename DType>
- MSHADOW_XINLINE static void Map(int i, int matrix_size, int stride, DType* data, bool to_lower) {
+ MSHADOW_XINLINE static void Map(int i, size_t matrix_size, int stride, DType* data, bool to_lower) {
// Below computation works even when we are dealing with a batch of matrices.
const int row((i % matrix_size) / stride), col(i % stride);
Review comment:
row int-> index_t
----------------------------------------------------------------
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] josephevans commented on a change in pull request #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
josephevans commented on a change in pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#discussion_r460159278
##########
File path: tests/nightly/test_large_array.py
##########
@@ -1167,6 +1168,32 @@ def check_correctness(mxnet_op, numpy_op, atol=1e-3):
check_gather()
check_binary_broadcast()
+def test_linalg():
+ def check_potrf():
+ # creating an identity matrix input
+ A = nd.zeros((LARGE_SQ_X, LARGE_SQ_X))
+ for i in range(LARGE_SQ_X):
+ A[i,i] = 1
+
+ out = nd.linalg.potrf(A)
+ # output should be an identity matrix
+ for i in range(LARGE_SQ_X):
+ assert out[i,i] == 1
+
+ def check_potri():
+ # creating an identity matrix input
+ A = nd.zeros((LARGE_SQ_X, LARGE_SQ_X))
+ for i in range(LARGE_SQ_X):
+ A[i,i] = 1
+
+ out = nd.linalg.potri(A)
+ # output should be an identity matrix
+ for i in range(LARGE_SQ_X):
Review comment:
I'll refactor this with a follow-up PR that includes backwards tests.
##########
File path: tests/nightly/test_large_array.py
##########
@@ -1167,6 +1168,32 @@ def check_correctness(mxnet_op, numpy_op, atol=1e-3):
check_gather()
check_binary_broadcast()
+def test_linalg():
+ def check_potrf():
+ # creating an identity matrix input
+ A = nd.zeros((LARGE_SQ_X, LARGE_SQ_X))
+ for i in range(LARGE_SQ_X):
+ A[i,i] = 1
+
+ out = nd.linalg.potrf(A)
+ # output should be an identity matrix
+ for i in range(LARGE_SQ_X):
Review comment:
I'll refactor this with a follow-up PR that includes backwards tests.
----------------------------------------------------------------
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 #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-663067039
Jenkins CI successfully triggered : [unix-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] szha merged pull request #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
szha merged pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752
----------------------------------------------------------------
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] josephevans commented on pull request #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
josephevans commented on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-662817291
@mxnet-bot run ci [website]
----------------------------------------------------------------
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] josephevans edited a comment on pull request #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
josephevans edited a comment on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-663618670
> Can you add the results by running
>
> ```
> pytest tests/nightly/test_large_array.py::<insert function_name>
> ```
>
Test results are already in the description. :)
> Also you could rework the tests so that you add backward & forward in the same call
> Similar to @mseth10 #18744
>
> Something like
>
> ```
> def run_<op>(inp):
> inp.attach_grad()
> with mx.autograd.record():
> out = mx.nd.linalg.<op>(inp)
> return inp.grad, out
> ```
I will follow up with an additional PR to add backward tests for these two operators.
----------------------------------------------------------------
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 #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-662817305
Jenkins CI successfully triggered : [website]
----------------------------------------------------------------
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] josephevans commented on pull request #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
josephevans commented on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-663618670
> Can you add the results by running
>
> ```
> pytest tests/nightly/test_large_array.py::<insert function_name>
> ```
>
> Also you could rework the tests so that you add backward & forward in the same call
> Similar to @mseth10 #18744
>
> Something like
>
> ```
> def run_<op>(inp):
> inp.attach_grad()
> with mx.autograd.record():
> out = mx.nd.linalg.<op>(inp)
> return inp.grad, out
> ```
Test results are already in the description. :)
----------------------------------------------------------------
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] josephevans commented on pull request #18752: Fix linalg_potri and linalg_potrf operators for large tensor.
Posted by GitBox <gi...@apache.org>.
josephevans commented on pull request #18752:
URL: https://github.com/apache/incubator-mxnet/pull/18752#issuecomment-663066996
@mxnet-bot run ci [unix-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