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/12 17:24:04 UTC
[GitHub] [tvm] vvchernov opened a new pull request, #13596: [Meta][TIR] Fix func registration issue during TIR kernel tuning by meta-scheduler
vvchernov opened a new pull request, #13596:
URL: https://github.com/apache/tvm/pull/13596
I investigated TIR kernel tuning by meta-scheduler and prepared for it the python script with minimal imports from tvm.
There is sketch of my code:
```
import tvm
from tvm import meta_schedule as ms
from tvm.script import tir as T
@tvm.script.ir_module
class MyModule:
@T.prim_func
def main(...):
...
if __name__ == "__main__":
# params init
...
database = ms.database.JSONDatabase(
workload_path,
record_path,
work_dir=work_dir,
module_equality="ignore-ndarray",
)
ms.tir_integration.tune_tir(
mod=MyModule,
target=target,
work_dir=work_dir,
max_trials_global=trials_number,
database=database,
strategy="replay-trace",
)
```
But it has failed due to [check](https://github.com/apache/tvm/blob/main/src/meta_schedule/space_generator/space_generator.cc#L28) on native side of space generator. It tried to get packed func from tvm.topi.x86.utils.target_has_vnni, but it is not registrated in my case.
Of course, the simplest fix is to add `from tvm.topi.x86.utils import target_has_vnni` or `from tvm import topi` or `from tvm import relay` to python tuning script. But as a client of TIR and meta scheduler high-level API I do not need to know internal context. As result I've fixed it inside TVM.
The main fix is `from tvm import relay` in ms.relay_integration module. I should note that it does it by its-self if TYPE_CHECKING=True (but looks like it always False). Other code is related to hidding `from tvm.meta_schedule import is_meta_schedule_enabled` due to circular python imports.
--
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 #13596: [Meta][TIR] Fix func registration issue during TIR kernel tuning by meta-scheduler
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13596:
URL: https://github.com/apache/tvm/pull/13596#issuecomment-1346919673
<!---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 @Hzfengsy, @junrushao <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] vvchernov commented on a diff in pull request #13596: [Meta][TIR] Fix func registration issue during TIR kernel tuning by meta-scheduler
Posted by GitBox <gi...@apache.org>.
vvchernov commented on code in PR #13596:
URL: https://github.com/apache/tvm/pull/13596#discussion_r1046775014
##########
python/tvm/meta_schedule/relay_integration.py:
##########
@@ -45,8 +45,7 @@
from .tune_context import TuneContext
from .utils import fork_seed
-if TYPE_CHECKING:
- from tvm import relay
+from tvm import relay
Review Comment:
Thank you @junrushao for your answer. Yes, we can import something locally (for example, during space generator initializing), but I do not think it is good solution as result. So, I tag WIP for this PR and waiting for ideas from [issue](https://github.com/apache/tvm/issues/13604) and/or [discussion](https://discuss.tvm.apache.org/t/failure-of-registration-of-global-function-in-tvm-python-api/14093). In one case, if topi-relay dependences will be avoided, I can import needed func from topi on meta-scheduler side. In another case, if global functions will be separated (for example, from topi.x86.topi), I use them without dependences on relay or topi.
Just now I use fix on my test script side, but I think it should be fixed as result in some correct way.
--
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] vvchernov commented on a diff in pull request #13596: [Meta][TIR] Fix func registration issue during TIR kernel tuning by meta-scheduler
Posted by GitBox <gi...@apache.org>.
vvchernov commented on code in PR #13596:
URL: https://github.com/apache/tvm/pull/13596#discussion_r1046205432
##########
python/tvm/meta_schedule/relay_integration.py:
##########
@@ -45,8 +45,7 @@
from .tune_context import TuneContext
from .utils import fork_seed
-if TYPE_CHECKING:
- from tvm import relay
+from tvm import relay
Review Comment:
Ok. But there is two things:
1. Meta-scheduler potentially can depend on relay if TYPE_CHECKING=True
2. This line fixed the issue, I cannot simply skip it
Of course I can use `from tvm.topi.x86.utils import target_has_vnni`, but it does not change anything due to topi depends on relay.
May be do you know another way?
--
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 commented on a diff in pull request #13596: [Meta][TIR] Fix func registration issue during TIR kernel tuning by meta-scheduler
Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #13596:
URL: https://github.com/apache/tvm/pull/13596#discussion_r1046337792
##########
python/tvm/meta_schedule/relay_integration.py:
##########
@@ -45,8 +45,7 @@
from .tune_context import TuneContext
from .utils import fork_seed
-if TYPE_CHECKING:
- from tvm import relay
+from tvm import relay
Review Comment:
> Meta-scheduler potentially can depend on relay if TYPE_CHECKING=True
This is a python's type checking mechanism, so you may assume "TYPE_CHECKING" is always "False" when the program is executed: https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING
> due to topi depends on relay
It is a sad fact and potentially a design issue. Ideally, TOPI should never depend on Relay in the first place, because it does not need to: TOPI + TE + TIR could do code generation without much problem at all. Let's start do things in a right way from new modules like MetaSchedule or Relax.
> Of course I can use from `tvm.topi.x86.utils import target_has_vnni`, but it does not change anything
We could import locally in a method in `relay_integration.py`, so that the importing won't happen until users are sure that they want to import Relay.
--
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 commented on a diff in pull request #13596: [Meta][TIR] Fix func registration issue during TIR kernel tuning by meta-scheduler
Posted by GitBox <gi...@apache.org>.
junrushao commented on code in PR #13596:
URL: https://github.com/apache/tvm/pull/13596#discussion_r1046149728
##########
python/tvm/meta_schedule/relay_integration.py:
##########
@@ -45,8 +45,7 @@
from .tune_context import TuneContext
from .utils import fork_seed
-if TYPE_CHECKING:
- from tvm import relay
+from tvm import relay
Review Comment:
By design, MetaSchedule should not depend on relay. Let's keep this line 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
Re: [PR] WIP: [Meta][TIR] Fix func registration issue during TIR kernel tuning by meta-scheduler [tvm]
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen closed pull request #13596: WIP: [Meta][TIR] Fix func registration issue during TIR kernel tuning by meta-scheduler
URL: https://github.com/apache/tvm/pull/13596
--
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