You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/07/18 23:22:13 UTC
[GitHub] [tvm] jwfromm opened a new pull request, #12124: [Relay][Op] Trilu operator implementation
jwfromm opened a new pull request, #12124:
URL: https://github.com/apache/tvm/pull/12124
This PR adds a new operator that supports triangular masking similar to that in `np.triu` and `np.tril`. The addition of `relay.trilu` conveniently lets us pass many of the remaining onnx 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mbrookhart merged pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
mbrookhart merged PR #12124:
URL: https://github.com/apache/tvm/pull/12124
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mbrookhart commented on pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on PR #12124:
URL: https://github.com/apache/tvm/pull/12124#issuecomment-1203148218
Thanks @jwfromm @sfvaroglu @mikepapadim @shingjan
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
jwfromm commented on PR #12124:
URL: https://github.com/apache/tvm/pull/12124#issuecomment-1196125814
I added pytorch testing and integration. Thanks for the recommendation @shingjan.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] sfvaroglu commented on pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
sfvaroglu commented on PR #12124:
URL: https://github.com/apache/tvm/pull/12124#issuecomment-1188998506
LGTM! Thanks for doing this @jwfromm! cc @mikepapadim
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on a diff in pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
jwfromm commented on code in PR #12124:
URL: https://github.com/apache/tvm/pull/12124#discussion_r930491682
##########
tests/python/frontend/onnx/test_forward.py:
##########
@@ -5241,23 +5241,7 @@ def verify_eyelike(indata, dynamic=False):
"test_training_dropout_mask",
"test_training_dropout_zero_ratio",
"test_training_dropout_zero_ratio_mask",
- "test_tril",
- "test_tril_pos",
- "test_tril_square",
- "test_tril_square_neg",
- "test_tril_neg",
- "test_tril_one_row_neg",
- "test_tril_out_neg",
- "test_tril_out_pos",
"test_tril_zero",
Review Comment:
Seems like it also doesnt work with nvptx for the same issue with empty tensors.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mbrookhart commented on a diff in pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on code in PR #12124:
URL: https://github.com/apache/tvm/pull/12124#discussion_r930088467
##########
tests/python/frontend/onnx/test_forward.py:
##########
@@ -5241,23 +5241,7 @@ def verify_eyelike(indata, dynamic=False):
"test_training_dropout_mask",
"test_training_dropout_zero_ratio",
"test_training_dropout_zero_ratio_mask",
- "test_tril",
- "test_tril_pos",
- "test_tril_square",
- "test_tril_square_neg",
- "test_tril_neg",
- "test_tril_one_row_neg",
- "test_tril_out_neg",
- "test_tril_out_pos",
"test_tril_zero",
Review Comment:
I haven't looked at this op at all. How tricky would it be to support the zero case? Otherwise LGTM.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a diff in pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
shingjan commented on code in PR #12124:
URL: https://github.com/apache/tvm/pull/12124#discussion_r930499403
##########
python/tvm/relay/op/transform.py:
##########
@@ -1889,3 +1889,46 @@ def stft(
window = _make.ones([n_fft], "int32")
return _make.stft(data, n_fft, hop_length, win_length, window, normalized, onesided)
+
+
+def trilu(data, k, upper=True):
+ """
+ Given a 2-D matrix or batches of 2-D matrices, returns the
+ upper or lower triangular part of the tensor.
+
+ Parameters
+ ----------
+ data: relay.Expr
+ The tensor that trilu will be applied to. Must be either
+ a 2D matrix or a tensor of batches of 2D matrices.
+
+ k: int
+ The number of diagonals above or below the main diagonal
+ to exclude or include.
+
+ upper: bool, optional
+ If True, only upper triangular values of input are kept,
+ if False, the lower triangular values are kept.
+
+
+ Returns
+ -------
+ ret : relay.Expr
+ The new tensor with appropriate diagonals set to zero.
+
+ Examples
+ --------
+ .. code-block:: python
+
+ x = [[0, 1, 2],
+ [3, 4, 5],
+ [6, 7, 8]]
+
+ relay.trilu(x, True, 0) =
+ [[0, 1, 2],
+ [0, 4, 5],
+ [0, 0, 8]]
+ """
+ if not isinstance(k, Expr):
Review Comment:
Nit: Should we check if k is `int`, do `const(k, "int32")` and throw an error otherwise? The reason I am asking is bcoz in the triu/tril op of pytorch `k` is actually an int instead of 0D tensor. Could cover the case of wrongly typed user input.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on a diff in pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
jwfromm commented on code in PR #12124:
URL: https://github.com/apache/tvm/pull/12124#discussion_r930491682
##########
tests/python/frontend/onnx/test_forward.py:
##########
@@ -5241,23 +5241,7 @@ def verify_eyelike(indata, dynamic=False):
"test_training_dropout_mask",
"test_training_dropout_zero_ratio",
"test_training_dropout_zero_ratio_mask",
- "test_tril",
- "test_tril_pos",
- "test_tril_square",
- "test_tril_square_neg",
- "test_tril_neg",
- "test_tril_one_row_neg",
- "test_tril_out_neg",
- "test_tril_out_pos",
"test_tril_zero",
Review Comment:
Seems like it also doesnt work with nvptx for the same issue with empty tensors. I'll add them here and see how it does in CI.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on a diff in pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
jwfromm commented on code in PR #12124:
URL: https://github.com/apache/tvm/pull/12124#discussion_r930506616
##########
python/tvm/relay/op/transform.py:
##########
@@ -1889,3 +1889,46 @@ def stft(
window = _make.ones([n_fft], "int32")
return _make.stft(data, n_fft, hop_length, win_length, window, normalized, onesided)
+
+
+def trilu(data, k, upper=True):
+ """
+ Given a 2-D matrix or batches of 2-D matrices, returns the
+ upper or lower triangular part of the tensor.
+
+ Parameters
+ ----------
+ data: relay.Expr
+ The tensor that trilu will be applied to. Must be either
+ a 2D matrix or a tensor of batches of 2D matrices.
+
+ k: int
+ The number of diagonals above or below the main diagonal
+ to exclude or include.
+
+ upper: bool, optional
+ If True, only upper triangular values of input are kept,
+ if False, the lower triangular values are kept.
+
+
+ Returns
+ -------
+ ret : relay.Expr
+ The new tensor with appropriate diagonals set to zero.
+
+ Examples
+ --------
+ .. code-block:: python
+
+ x = [[0, 1, 2],
+ [3, 4, 5],
+ [6, 7, 8]]
+
+ relay.trilu(x, True, 0) =
+ [[0, 1, 2],
+ [0, 4, 5],
+ [0, 0, 8]]
+ """
+ if not isinstance(k, Expr):
Review Comment:
If it's an `int`, then it will already be cast as a const (since its not an `Expr`).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
jwfromm commented on PR #12124:
URL: https://github.com/apache/tvm/pull/12124#issuecomment-1203130277
@mbrookhart I think this is ready to 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on a diff in pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
jwfromm commented on code in PR #12124:
URL: https://github.com/apache/tvm/pull/12124#discussion_r930487887
##########
tests/python/frontend/onnx/test_forward.py:
##########
@@ -5241,23 +5241,7 @@ def verify_eyelike(indata, dynamic=False):
"test_training_dropout_mask",
"test_training_dropout_zero_ratio",
"test_training_dropout_zero_ratio_mask",
- "test_tril",
- "test_tril_pos",
- "test_tril_square",
- "test_tril_square_neg",
- "test_tril_neg",
- "test_tril_one_row_neg",
- "test_tril_out_neg",
- "test_tril_out_pos",
"test_tril_zero",
Review Comment:
It actually works on llvm and cuda. I was testing on my macbook and it seems like the metal backend in general doesnt support empty tensors. I think for CI we could add these cases.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mikepapadim commented on pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
mikepapadim commented on PR #12124:
URL: https://github.com/apache/tvm/pull/12124#issuecomment-1189241324
thanks @jwfromm, if this one is merged then we should close following as depreceated #11761 #10873 and #9329
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
jwfromm commented on PR #12124:
URL: https://github.com/apache/tvm/pull/12124#issuecomment-1188429317
@sfvaroglu Can you take a look at this 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on pull request #12124: [Relay][Op] Trilu operator implementation
Posted by GitBox <gi...@apache.org>.
jwfromm commented on PR #12124:
URL: https://github.com/apache/tvm/pull/12124#issuecomment-1196929853
Unfortunately the empty tensor tests still fail on CI GPUs. I'm not sure why, it doesnt seem like its directly related to this PR so I'm going to reenable skips for those 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org