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/04/21 10:19:24 UTC

[GitHub] [tvm] pfk-beta opened a new pull request, #11089: change hardcoded names of loggers to `__name__`

pfk-beta opened a new pull request, #11089:
URL: https://github.com/apache/tvm/pull/11089

   This is PR related to RFC posted here: https://discuss.tvm.apache.org/t/pre-rfc-generic-logger-names-in-python/12611 . I've changed hardcoded logger names to generic `__name__`.  I've changed most of the examples, but few I didn't touch - let's discuss them, because I'm not sure wether change it. 
   
   Points to discuss:
   1. `./autotvm/graph_tuner/base_graph_tuner.py:        self._logger = logging.getLogger(name + "_logger")`
   2. `./meta_schedule/testing/tune_te_meta_schedule.py:logging.getLogger("tvm.meta_schedule").setLevel(logging.DEBUG)`
   3. `./meta_schedule/testing/tune_relay_meta_schedule.py:logging.getLogger("tvm.meta_schedule").setLevel(logging.DEBUG)`
   4. `./driver/tvmc/main.py:    logging.getLogger("tvm.driver.tvmc").setLevel(40 - args.verbose * 10)`


-- 
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] junrushao1994 commented on pull request #11089: change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1105671606

   @pfk-beta I can answer your question in MetaSchedule:
   
   > ./meta_schedule/testing/tune_te_meta_schedule.py:logging.getLogger("tvm.meta_schedule").setLevel(logging.DEBUG)
   > ./meta_schedule/testing/tune_relay_meta_schedule.py:logging.getLogger("tvm.meta_schedule").setLevel(logging.DEBUG)
   
   These two are independent files, which are not imported to the system by default; Both are supposed to be run from command line for demo usage. Therefore, it makes sense to leave it as it is


-- 
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] pfk-beta commented on pull request #11089: [PY] change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
pfk-beta commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1111972401

   > Well..I think we could take a series of incremental steps towards this goal. If there is anything uncertain in some particular places like you mentioned, we might defer those to a second PR to figure out, and use this first PR to clear the path for easier ones
   
   I think, we should look closer at loggers in GraphTuners, and decide how should look mechanism for logging there - imho additional PR is not needed.


-- 
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] junrushao1994 commented on pull request #11089: [PY] change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1111716982

   Well..I think we could take a series of incremental steps towards this goal. If there is anything uncertain in some particular places like you mentioned, we might defer those to a second PR to figure out, and use this first PR to clear the path for easier ones


-- 
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] junrushao1994 commented on pull request #11089: change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1105670228

   Let's leave this PR open for 1 week or 2 just in case there is any legit concern :-)
   
   


-- 
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] pfk-beta commented on pull request #11089: [PY] change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
pfk-beta commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1121013478

   I'm not fluend with your CI so I have rebased branch on to of main - maybe the problem is old version of main. I have pushed changes with flag `--force-with-lease`. I hope it will help


-- 
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] pfk-beta commented on pull request #11089: change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
pfk-beta commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1106231831

   @jroesch @kevinyuan I can see that you contributed to the codebase of BaseGraphTuner, and logging mechanism there. Can you describe, if you remember that (2-3 years ago...), what was the intent for such configuration?


-- 
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] junrushao1994 commented on a diff in pull request #11089: change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11089:
URL: https://github.com/apache/tvm/pull/11089#discussion_r855420759


##########
docs/arch/relay_op_strategy.rst:
##########
@@ -278,5 +278,5 @@ model to learn which implementation is used for each operator.
 
 .. code:: python
 
-    logging.getLogger("te_compiler").setLevel(logging.INFO)
-    logging.getLogger("te_compiler").addHandler(logging.StreamHandler(sys.stdout))
+    logging.getLogger("tvm.relay.backend.te_compiler").setLevel(logging.INFO)

Review Comment:
   i'm not an expert in our doc rendering, but looks like `__name__` doesn't work in this case? if so, this change looks good to me



-- 
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] junrushao1994 commented on pull request #11089: change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1105497550

   Not sure why the tests are failing :-(


-- 
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] pfk-beta commented on pull request #11089: change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
pfk-beta commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1105023360

   @areusch @junrushao1994 review needed


-- 
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] pfk-beta commented on pull request #11089: change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
pfk-beta commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1106218397

   > Not sure why the tests are failing :-(
   
   I'm not expert in reading test output of TVM, but when I open full logs(https://ci.tlcpack.ai/blue/rest/organizations/jenkins/pipelines/tvm/branches/PR-11089/runs/1/nodes/348/steps/786/log/?start=0) I can see on this timestamp (2022-04-21T11:25:28.044Z) huger error, here is only part:
   ```
   [2022-04-21T11:25:28.044Z] DEBUG:tvm.autotvm.tuner.tuner:No: 19	GFLOPS: 0.00/93.74	result: Traceback (most recent call last):
   [2022-04-21T11:25:28.044Z]   File "/workspace/python/tvm/autotvm/measure/measure_methods.py", line 721, in __call__
   [2022-04-21T11:25:28.044Z]     yield remote, remote.load_module(os.path.split(build_result.filename)[1])
   [2022-04-21T11:25:28.044Z]   File "/workspace/python/tvm/autotvm/measure/measure_methods.py", line 685, in run_through_rpc
   [2022-04-21T11:25:28.044Z]     costs = time_f(*args).results
   [2022-04-21T11:25:28.044Z]   File "/workspace/python/tvm/runtime/module.py", line 297, in evaluator
   [2022-04-21T11:25:28.044Z]     blob = feval(*args)
   [2022-04-21T11:25:28.044Z]   File "tvm/_ffi/_cython/./packed_func.pxi", line 323, in tvm._ffi._cy3.core.PackedFuncBase.__call__
   [2022-04-21T11:25:28.044Z]   File "tvm/_ffi/_cython/./packed_func.pxi", line 257, in tvm._ffi._cy3.core.FuncCall
   [2022-04-21T11:25:28.044Z]   File "tvm/_ffi/_cython/./packed_func.pxi", line 246, in tvm._ffi._cy3.core.FuncCall3
   [2022-04-21T11:25:28.044Z]   File "tvm/_ffi/_cython/./base.pxi", line 163, in tvm._ffi._cy3.core.CALL
   [2022-04-21T11:25:28.044Z] tvm._ffi.base.TVMError: Traceback (most recent call last):
   [2022-04-21T11:25:28.044Z]   4: TVMFuncCall
   [2022-04-21T11:25:28.044Z]         at /workspace/src/runtime/c_runtime_api.cc:477
   [2022-04-21T11:25:28.044Z]   3: tvm::runtime::PackedFuncObj::CallPacked(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*) const
   [2022-04-21T11:25:28.044Z]         at /workspace/include/tvm/runtime/packed_func.h:1217
   [2022-04-21T11:25:28.044Z]   2: tvm::runtime::RPCWrappedFunc::operator()(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*) const
   [2022-04-21T11:25:28.044Z]         at /workspace/src/runtime/rpc/rpc_module.cc:127
   [2022-04-21T11:25:28.044Z]   1: tvm::runtime::RPCClientSession::CallFunc(void*, TVMValue const*, int const*, int, std::function<void (tvm::runtime::TVMArgs)> const&)
   [2022-04-21T11:25:28.044Z]         at /workspace/src/runtime/rpc/rpc_endpoint.cc:1009
   [2022-04-21T11:25:28.044Z]   0: tvm::runtime::RPCEndpoint::CallFunc(void*, TVMValue const*, int const*, int, std::function<void (tvm::runtime::TVMArgs)>)
   [2022-04-21T11:25:28.044Z]         at /workspace/src/runtime/rpc/rpc_endpoint.cc:801
   [2022-04-21T11:25:28.044Z]   File "/workspace/src/runtime/rpc/rpc_endpoint.cc", line 801
   [2022-04-21T11:25:28.044Z] TVMError: 
   [2022-04-21T11:25:28.044Z] ---------------------------------------------------------------
   [2022-04-21T11:25:28.044Z] An error occurred during the execution of TVM.
   [2022-04-21T11:25:28.044Z] For more information, please see: https://tvm.apache.org/docs/errors.html
   [2022-04-21T11:25:28.044Z] ---------------------------------------------------------------
   [2022-04-21T11:25:28.044Z]   Check failed: (code == RPCCode::kReturn) is false: code=kShutdown
   [2022-04-21T11:25:28.044Z] 
   [2022-04-21T11:25:28.044Z] During handling of the above exception, another exception occurred:
   ```
   
   
   
   
   
   
   > Let's leave this PR open for 1 week or 2 just in case there is any legit concern :-)
   
   I think 05.05 may be good date for merging :D
   
   
   
   > 1. ./autotvm/graph_tuner/base_graph_tuner.py: self._logger = logging.getLogger(name + "_logger")
   ./meta_schedule/testing
   
   BaseGraphTuner which has such construction, is inherited by 2 different GraphTuners, which are located in separate files. I suggest to use following logger name: `__name__ + "__" + name`, or remove `_logger` field from `BaseGraphTuner` and its children.
   


-- 
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] pfk-beta commented on a diff in pull request #11089: change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
pfk-beta commented on code in PR #11089:
URL: https://github.com/apache/tvm/pull/11089#discussion_r855961554


##########
docs/arch/relay_op_strategy.rst:
##########
@@ -278,5 +278,5 @@ model to learn which implementation is used for each operator.
 
 .. code:: python
 
-    logging.getLogger("te_compiler").setLevel(logging.INFO)
-    logging.getLogger("te_compiler").addHandler(logging.StreamHandler(sys.stdout))
+    logging.getLogger("tvm.relay.backend.te_compiler").setLevel(logging.INFO)

Review Comment:
   Me too, I think it is example script in which user is configuring logging, so I just updated old logger name to new one.



-- 
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] areusch commented on pull request #11089: [PY] change hardcoded names of loggers to `__name__`

Posted by GitBox <gi...@apache.org>.
areusch commented on PR #11089:
URL: https://github.com/apache/tvm/pull/11089#issuecomment-1124196823

   @pfk-beta yep, i think you may need to adjust some logger values in tests as well. take a look at the tests view, which coudl help. https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-11089/6/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