You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "ekalda (via GitHub)" <gi...@apache.org> on 2023/03/02 15:29:40 UTC
[GitHub] [tvm] ekalda opened a new pull request, #14175: [TVMC] Fix logging in TVMC
ekalda opened a new pull request, #14175:
URL: https://github.com/apache/tvm/pull/14175
Three logger related changes in this patch:
* Currently we don't set the output stream on the Python logger, so it defaults to sys.stderr, which means we only get some logger output when the command fails. So set the output stream to sys.stdout
* Currently we can add -v flag to anywhere in the command line for `tvmc compile`, but only between `tvmc` and `run`/`tune` for `run` and `tune`. Unify the behaviour such that we can add the flag anywhere on the command line.
* Set the effective upper bound of `-v`s to 3 as 4 could result in NOTSET which would not output anything.
--
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] ekalda commented on a diff in pull request #14175: [TVMC] Fix logging in TVMC
Posted by "ekalda (via GitHub)" <gi...@apache.org>.
ekalda commented on code in PR #14175:
URL: https://github.com/apache/tvm/pull/14175#discussion_r1135777206
##########
tests/python/driver/tvmc/test_command_line.py:
##########
@@ -210,3 +210,45 @@ def test_tvmc_compile_input_model(mock_compile_model, tmpdir_factory, model):
_main(run_arg)
mock_compile_model.assert_called_once()
+
+
+# TODO(ekalda): This test just makes sure that we log - unfortunately pytest seems to intercept
Review Comment:
Good call! Added a test...
--
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 merged pull request #14175: [TVMC] Fix logging in TVMC
Posted by "lhutton1 (via GitHub)" <gi...@apache.org>.
lhutton1 merged PR #14175:
URL: https://github.com/apache/tvm/pull/14175
--
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] ekalda commented on pull request #14175: [TVMC] Fix logging in TVMC
Posted by "ekalda (via GitHub)" <gi...@apache.org>.
ekalda commented on PR #14175:
URL: https://github.com/apache/tvm/pull/14175#issuecomment-1468364272
Sorry this fell off my radar for a bit - updated the patch now with a test to check that we set the `logging.basicConfig`.
--
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 #14175: [TVMC] Fix logging in TVMC
Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #14175:
URL: https://github.com/apache/tvm/pull/14175#issuecomment-1452060177
<!---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-->
* cc @Mousius, @gromero, @leandron, @lhutton1 <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] lhutton1 commented on a diff in pull request #14175: [TVMC] Fix logging in TVMC
Posted by "lhutton1 (via GitHub)" <gi...@apache.org>.
lhutton1 commented on code in PR #14175:
URL: https://github.com/apache/tvm/pull/14175#discussion_r1123810642
##########
tests/python/driver/tvmc/test_command_line.py:
##########
@@ -210,3 +210,45 @@ def test_tvmc_compile_input_model(mock_compile_model, tmpdir_factory, model):
_main(run_arg)
mock_compile_model.assert_called_once()
+
+
+# TODO(ekalda): This test just makes sure that we log - unfortunately pytest seems to intercept
Review Comment:
Yeah I think pytest overwrites the logging handlers :( I think you could somewhat check this with mocking, at least check that `logging.basicConfig(stream=sys.stdout)` was called with the correct arguments - there's a similar example here https://github.com/apache/tvm/blob/23ade0c14b29c2c2710f6580035878f130eea52b/tests/python/driver/tvmc/test_transform.py#L50
--
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 diff in pull request #14175: [TVMC] Fix logging in TVMC
Posted by "lhutton1 (via GitHub)" <gi...@apache.org>.
lhutton1 commented on code in PR #14175:
URL: https://github.com/apache/tvm/pull/14175#discussion_r1123810642
##########
tests/python/driver/tvmc/test_command_line.py:
##########
@@ -210,3 +210,45 @@ def test_tvmc_compile_input_model(mock_compile_model, tmpdir_factory, model):
_main(run_arg)
mock_compile_model.assert_called_once()
+
+
+# TODO(ekalda): This test just makes sure that we log - unfortunately pytest seems to intercept
Review Comment:
Yeah I think pytest overwrites the logging handlers :( I think you could somewhat check this with mocking, at least check that `logging.basicConfig()` was called with the correct arguments - there's a similar example here https://github.com/apache/tvm/blob/23ade0c14b29c2c2710f6580035878f130eea52b/tests/python/driver/tvmc/test_transform.py#L50
--
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 pull request #14175: [TVMC] Fix logging in TVMC
Posted by "lhutton1 (via GitHub)" <gi...@apache.org>.
lhutton1 commented on PR #14175:
URL: https://github.com/apache/tvm/pull/14175#issuecomment-1470255441
Thanks @ekalda, @leandron!
--
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