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/03/07 16:03:47 UTC
[GitHub] [tvm] lhutton1 opened a new pull request #10518: [AOT] Use python temporary directory for AOT tests
lhutton1 opened a new pull request #10518:
URL: https://github.com/apache/tvm/pull/10518
Uses a python temporary directory with a context manager in an effort to solve the flaky FVP tests raised in #10300 and #10314. Now that CI is becoming more and more parallelized, the thinking is that the python temporary directory implementation might be more stable than `utils.tempdir`. Removing the XFail markings off the affected tests, but keeping the work around implemented in #10408 while we monitor with the above change.
cc @manupa-arm @ekalda @grant-arm @Mousius
--
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] manupa-arm commented on pull request #10518: [AOT] Use python temporary directory for AOT tests
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #10518:
URL: https://github.com/apache/tvm/pull/10518#issuecomment-1063710525
Thanks @lhutton1 . This is merged now!
--
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] lhutton1 commented on a change in pull request #10518: [AOT] Use python temporary directory for AOT tests
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10518:
URL: https://github.com/apache/tvm/pull/10518#discussion_r820855366
##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -731,130 +733,129 @@ def run_and_check(
This method uses the original test data and compiled runtime.Modules
to run in the test runner to verify the results.
"""
+ tmpdir = tempfile.TemporaryDirectory() if test_dir is None else contextlib.surpress()
+ with tmpdir:
+ base_path = os.path.join(tmpdir.name, "test") if test_dir is None else test_dir
Review comment:
Here is the actual change... the diff looks funny because of the indentation required
--
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] manupa-arm merged pull request #10518: [AOT] Use python temporary directory for AOT tests
Posted by GitBox <gi...@apache.org>.
manupa-arm merged pull request #10518:
URL: https://github.com/apache/tvm/pull/10518
--
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] lhutton1 commented on a change in pull request #10518: [AOT] Use python temporary directory for AOT tests
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10518:
URL: https://github.com/apache/tvm/pull/10518#discussion_r821641635
##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -731,130 +733,129 @@ def run_and_check(
This method uses the original test data and compiled runtime.Modules
to run in the test runner to verify the results.
"""
+ tmpdir = tempfile.TemporaryDirectory() if test_dir is None else contextlib.surpress()
Review comment:
Yeah this was an attempt to make the `with` statement conditional, but without limiting us to Python 3.7 (in which case `nullcontext` could have been used), see https://stackoverflow.com/a/45187287. I agree it looks a bit strange, I was trying to avoid wrapping the body in another function. In hindsight it probably would be more readable to do that (wrap the body in a function) though.
--
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] lhutton1 commented on a change in pull request #10518: [AOT] Use python temporary directory for AOT tests
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10518:
URL: https://github.com/apache/tvm/pull/10518#discussion_r821641635
##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -731,130 +733,129 @@ def run_and_check(
This method uses the original test data and compiled runtime.Modules
to run in the test runner to verify the results.
"""
+ tmpdir = tempfile.TemporaryDirectory() if test_dir is None else contextlib.surpress()
Review comment:
Yeah this was an attempt to make the `with` statement conditional, but without limiting us to Python 3.7 (in which case `nullcontext` could have been used), see https://stackoverflow.com/a/45187287. I agree it looks a bit strange, I was trying to avoid wrapping the body in another function. In hindsight it probably would be more readable to do that though.
--
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] lhutton1 commented on a change in pull request #10518: [AOT] Use python temporary directory for AOT tests
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on a change in pull request #10518:
URL: https://github.com/apache/tvm/pull/10518#discussion_r821641635
##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -731,130 +733,129 @@ def run_and_check(
This method uses the original test data and compiled runtime.Modules
to run in the test runner to verify the results.
"""
+ tmpdir = tempfile.TemporaryDirectory() if test_dir is None else contextlib.surpress()
Review comment:
Yeah this was an attempt to make the `with` statement conditional, but without limiting us to Python 3.7 (in which case `nullcontext` could have been used), see https://stackoverflow.com/a/45187287. I agree it looks a bit strange, I was trying to avoid wrapping the body in another function. In hindsight it probably would be more readable to do that (wrap the body in a function) though. Thanks!
--
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] manupa-arm commented on a change in pull request #10518: [AOT] Use python temporary directory for AOT tests
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #10518:
URL: https://github.com/apache/tvm/pull/10518#discussion_r821609823
##########
File path: tests/python/relay/aot/aot_test_utils.py
##########
@@ -731,130 +733,129 @@ def run_and_check(
This method uses the original test data and compiled runtime.Modules
to run in the test runner to verify the results.
"""
+ tmpdir = tempfile.TemporaryDirectory() if test_dir is None else contextlib.surpress()
Review comment:
Should not this be https://docs.python.org/3/library/contextlib.html#contextlib.suppress ?
Also why do we need to suppress execeptions here ?
I think a slightly better approach would be to make the body of "with" to be wrapped in a function that accepts base_path, where the dir could be provided within a "with" if test_dir is None. WDYT ?
--
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