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/12/18 07:45:59 UTC
[GitHub] [tvm] lightzhan-intellif opened a new pull request, #13640: [BugFix][TVMScript]fix var capturing order error.
lightzhan-intellif opened a new pull request, #13640:
URL: https://github.com/apache/tvm/pull/13640
This PR try to fix the following bug:
```python
def test_var_capturing_order():
b = 2
@T.prim_func
def test_case():
k: T.int32 = b
if __name__ == "__main__":
b = 1
```
In the prim func `test_case`, the vaule of b should be 2, rather than 1. The parser wrongly uses global vars to shadow the value of nonlocal vars, which should be reversed.
--
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] lightzhan-intellif commented on a diff in pull request #13640: [BugFix][TVMScript]fix var capturing order error.
Posted by GitBox <gi...@apache.org>.
lightzhan-intellif commented on code in PR #13640:
URL: https://github.com/apache/tvm/pull/13640#discussion_r1051557843
##########
tests/python/unittest/test_tvmscript_regression.py:
##########
@@ -58,7 +58,24 @@ def func_ref():
tvm.ir.assert_structural_equal(test_case, func_ref)
+def test_var_capturing_order():
+ b = 2
+
+ @T.prim_func
+ def test_case():
+ k: T.int32 = b
+
+ @T.prim_func
+ def func_ref():
+ k: T.int32 = 2
+ T.evaluate(0)
+
+ tvm.ir.assert_structural_equal(test_case, func_ref)
+
+
if __name__ == "__main__":
+ b = 1
Review Comment:
Yea, I've moved it!
> what is the behavior when there are nested closures?
I just had a test:
```python
def test_var():
a = 1
def test_var_0():
a = 2
@T.prim_func
def test_case():
k: T.int32 = a
return test_case
print(test_var_0().script())
```
Call `test_var` and get the output:
```
# from tvm.script import tir as T
@T.prim_func
def func():
# body
k: T.int32 = 2
T.evaluate(0)
```
It works as our expectation!
--
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] Hzfengsy commented on pull request #13640: [BugFix][TVMScript]fix var capturing order error.
Posted by GitBox <gi...@apache.org>.
Hzfengsy commented on PR #13640:
URL: https://github.com/apache/tvm/pull/13640#issuecomment-1356723917
cc @junrushao
--
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] wrongtest-intellif commented on a diff in pull request #13640: [BugFix][TVMScript]fix var capturing order error.
Posted by GitBox <gi...@apache.org>.
wrongtest-intellif commented on code in PR #13640:
URL: https://github.com/apache/tvm/pull/13640#discussion_r1051548626
##########
tests/python/unittest/test_tvmscript_regression.py:
##########
@@ -58,7 +58,24 @@ def func_ref():
tvm.ir.assert_structural_equal(test_case, func_ref)
+def test_var_capturing_order():
+ b = 2
+
+ @T.prim_func
+ def test_case():
+ k: T.int32 = b
+
+ @T.prim_func
+ def func_ref():
+ k: T.int32 = 2
+ T.evaluate(0)
+
+ tvm.ir.assert_structural_equal(test_case, func_ref)
+
+
if __name__ == "__main__":
+ b = 1
Review Comment:
Could we put it just before line `def test_var_capturing_order():` thus people could know what is the purpose this assignment is for. Also just for my curiosity, what is the behavior when there are nested closures?
--
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] lightzhan-intellif commented on a diff in pull request #13640: [BugFix][TVMScript]fix var capturing order error.
Posted by GitBox <gi...@apache.org>.
lightzhan-intellif commented on code in PR #13640:
URL: https://github.com/apache/tvm/pull/13640#discussion_r1051557843
##########
tests/python/unittest/test_tvmscript_regression.py:
##########
@@ -58,7 +58,24 @@ def func_ref():
tvm.ir.assert_structural_equal(test_case, func_ref)
+def test_var_capturing_order():
+ b = 2
+
+ @T.prim_func
+ def test_case():
+ k: T.int32 = b
+
+ @T.prim_func
+ def func_ref():
+ k: T.int32 = 2
+ T.evaluate(0)
+
+ tvm.ir.assert_structural_equal(test_case, func_ref)
+
+
if __name__ == "__main__":
+ b = 1
Review Comment:
Yea, I've moved it!
> what is the behavior when there are nested closures?
I just had a test:
```python
def test_var():
a = 1
def test_var_0():
a = 2
@T.prim_func
def test_case():
k: T.int32 = a
return test_case
print(test_var_0().script())
```
Call `test_var` and get the output:
```python
# from tvm.script import tir as T
@T.prim_func
def func():
# body
k: T.int32 = 2
T.evaluate(0)
```
It works as our expectation!
--
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] lightzhan-intellif commented on pull request #13640: [BugFix][TVMScript]fix var capturing order error
Posted by GitBox <gi...@apache.org>.
lightzhan-intellif commented on PR #13640:
URL: https://github.com/apache/tvm/pull/13640#issuecomment-1356994913
@tvm-bot rerun
--
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] tvm-bot commented on pull request #13640: [BugFix][TVMScript]fix var capturing order error.
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13640:
URL: https://github.com/apache/tvm/pull/13640#issuecomment-1356723686
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* No users to tag found in teams: `bugfix`, `tvmscript` <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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] wrongtest-intellif commented on a diff in pull request #13640: [BugFix][TVMScript]fix var capturing order error.
Posted by GitBox <gi...@apache.org>.
wrongtest-intellif commented on code in PR #13640:
URL: https://github.com/apache/tvm/pull/13640#discussion_r1051548626
##########
tests/python/unittest/test_tvmscript_regression.py:
##########
@@ -58,7 +58,24 @@ def func_ref():
tvm.ir.assert_structural_equal(test_case, func_ref)
+def test_var_capturing_order():
+ b = 2
+
+ @T.prim_func
+ def test_case():
+ k: T.int32 = b
+
+ @T.prim_func
+ def func_ref():
+ k: T.int32 = 2
+ T.evaluate(0)
+
+ tvm.ir.assert_structural_equal(test_case, func_ref)
+
+
if __name__ == "__main__":
+ b = 1
Review Comment:
Could we put it just before line `def test_var_capturing_order():` thus people could know what is the purpose this assignment is for. Also just for my curiosity, what the behavior when there are nested closures?
--
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] junrushao merged pull request #13640: [BugFix][TVMScript]fix var capturing order error
Posted by GitBox <gi...@apache.org>.
junrushao merged PR #13640:
URL: https://github.com/apache/tvm/pull/13640
--
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