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/09/28 14:13:22 UTC
[GitHub] [tvm] Lunderberg opened a new pull request, #12926: [TE] Raise error for non-bijective transformation
Lunderberg opened a new pull request, #12926:
URL: https://github.com/apache/tvm/pull/12926
This is a fix for a bug introduced in https://github.com/apache/tvm/pull/12904. Prior to then, an exception was raised when the transformation wouldn't be bijective over the transformed buffer's shape. The PR replaced the bijective check done as part of `DetectIterMap` with a check done on the returned `padding_predicate`. However, this check was not equivalent, and some transformations could erroneously apply, rather than raising an exception as being non-bijective.
This commit re-enables the bijectivity check in `DetectIterMap`, and adds a test case for this behavior.
--
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] vinx13 merged pull request #12926: [TE] Raise error for non-bijective transformation
Posted by GitBox <gi...@apache.org>.
vinx13 merged PR #12926:
URL: https://github.com/apache/tvm/pull/12926
--
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] echuraev commented on a diff in pull request #12926: [TE] Raise error for non-bijective transformation
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #12926:
URL: https://github.com/apache/tvm/pull/12926#discussion_r983058281
##########
tests/python/unittest/test_transform_layout.py:
##########
@@ -575,5 +575,18 @@ def test_size_one_buffer(shape, transform):
s[B].transform_layout(transform)
+def test_non_divisible_transform_raises_error():
+ A = te.placeholder([1, 3, 8, 8])
+ B = te.compute(A.shape, lambda *indices: A[indices])
+ s = te.create_schedule(B.op)
+
+ transform = lambda n, c, h, w: [n, c // 4, h, w, c % 4]
+ # Error occurs here, because the transformation would introduce
+ # padding. Padded transforms are supported in TIR-based
+ # schedules.
+ with pytest.raises(tvm.TVMError):
+ s[B].transform_layout(transform)
+
+
Review Comment:
Got it. Thank you. I agree.
--
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] Lunderberg commented on a diff in pull request #12926: [TE] Raise error for non-bijective transformation
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on code in PR #12926:
URL: https://github.com/apache/tvm/pull/12926#discussion_r982674487
##########
tests/python/unittest/test_transform_layout.py:
##########
@@ -575,5 +575,18 @@ def test_size_one_buffer(shape, transform):
s[B].transform_layout(transform)
+def test_non_divisible_transform_raises_error():
+ A = te.placeholder([1, 3, 8, 8])
+ B = te.compute(A.shape, lambda *indices: A[indices])
+ s = te.create_schedule(B.op)
+
+ transform = lambda n, c, h, w: [n, c // 4, h, w, c % 4]
+ # Error occurs here, because the transformation would introduce
+ # padding. Padded transforms are supported in TIR-based
+ # schedules.
+ with pytest.raises(tvm.TVMError):
+ s[B].transform_layout(transform)
+
+
Review Comment:
It could, but I think it would make the test less readable as an example use case, specifically what behavior is being tested, because the desired behavior differs in each case. It would look something like below, but there's nothing to call attention to the fact that `is_valid` changes the expected behavior, and isn't just a parameter being used in the setup.
```python
shape, transform, is_valid = tvm.testing.parameters(
([1, 8], lambda n, i: [i, n], True),
([1, 1, 8], lambda i, j, k: [j, te.AXIS_SEPARATOR, i, k], True),
([1, 1, 8], lambda i, j, k: [i, te.AXIS_SEPARATOR, j, k], True),
([1, 3, 8, 8], lambda i, j, k: [i, te.AXIS_SEPARATOR, j, k], False),
)
def test_transform_validity(shape, transform, is_valid):
dtype = "int8"
A = te.placeholder(shape, dtype, name="A")
B = te.compute(
shape=A.shape,
fcompute=lambda *indices: A[indices].astype(dtype),
name="B",
)
s = te.create_schedule(B.op)
if is_valid:
s[B].transform_layout(transform)
else:
with pytest.raises(tvm.TVMError):
s[B].transform_layout(transform)
```
--
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] echuraev commented on a diff in pull request #12926: [TE] Raise error for non-bijective transformation
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #12926:
URL: https://github.com/apache/tvm/pull/12926#discussion_r982600356
##########
tests/python/unittest/test_transform_layout.py:
##########
@@ -575,5 +575,18 @@ def test_size_one_buffer(shape, transform):
s[B].transform_layout(transform)
+def test_non_divisible_transform_raises_error():
+ A = te.placeholder([1, 3, 8, 8])
+ B = te.compute(A.shape, lambda *indices: A[indices])
+ s = te.create_schedule(B.op)
+
+ transform = lambda n, c, h, w: [n, c // 4, h, w, c % 4]
+ # Error occurs here, because the transformation would introduce
+ # padding. Padded transforms are supported in TIR-based
+ # schedules.
+ with pytest.raises(tvm.TVMError):
+ s[B].transform_layout(transform)
+
+
Review Comment:
nit: Probably we can joint this test case with `test_size_one_buffer`? Just extend its testing parameters
--
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