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/11/22 17:41:57 UTC

[GitHub] [tvm] tkonolige opened a new pull request, #13470: [MetaSchedule] Use current pass context in compile_relay, extract_tasks

tkonolige opened a new pull request, #13470:
URL: https://github.com/apache/tvm/pull/13470

   Adds the pass config information necessary for tuning and compiling relay with metaschedule to the existing pass context instead of overriding the existing one. Allows users to pass in their own pass instruments, required passes, and disabled passes. This also keeps the same API used to compile relay with autotvm and auto_scheduler.


-- 
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] masahi commented on pull request #13470: [MetaSchedule] Use current pass context in compile_relay, extract_tasks

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

   @junrushao Isn't it actually simplifying the API? Also considering that this changes makes the MS API consistent with other tuner APIs, I think it is a good change.


-- 
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 #13470: [MetaSchedule] Use current pass context in compile_relay, extract_tasks

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

   <!---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, @elvin-n, @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] masahi commented on a diff in pull request #13470: [MetaSchedule] Use current pass context in compile_relay, extract_tasks

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


##########
python/tvm/meta_schedule/relay_integration.py:
##########
@@ -390,16 +353,20 @@ def compile_relay(
     from tvm import relay
 
     # pylint: enable=import-outside-toplevel
-    mod, target, params, pass_config, executor = _normalize_params(
-        mod, target, params, pass_config, executor
-    )
+    mod, target, params, executor = _normalize_params(mod, target, params, executor)
+    pass_ctx = transform.PassContext.current()
+    pass_config = dict(pass_ctx.config)

Review Comment:
   To prevent breaking user's script after this change, how about setting a default ctx with opt_level = 3 if there is no current ctx?



-- 
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 pull request #13470: [MetaSchedule] Use current pass context in compile_relay, extract_tasks

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

   They are intentionally designed this way to keep the primary APIs as simple as possible, for advanced usecases like you mentioned in this PR, please do not use `compile_relay` and `extract_tasks` directly, but assemble those APIs call yourself. That is being said, I am not in favor of this change


-- 
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] masahi commented on pull request #13470: [MetaSchedule] Use current pass context in compile_relay, extract_tasks

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

   This also makes PRs like https://github.com/apache/tvm/pull/13688 and https://github.com/apache/tvm/pull/13659 unnecessary. 
   
   If we want to make the tuning API as simple as possible, we should remove all those default arguments (builder / runner / db etc, and even random seeds).


-- 
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] tkonolige commented on a diff in pull request #13470: [MetaSchedule] Use current pass context in compile_relay, extract_tasks

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


##########
python/tvm/meta_schedule/relay_integration.py:
##########
@@ -390,16 +353,20 @@ def compile_relay(
     from tvm import relay
 
     # pylint: enable=import-outside-toplevel
-    mod, target, params, pass_config, executor = _normalize_params(
-        mod, target, params, pass_config, executor
-    )
+    mod, target, params, executor = _normalize_params(mod, target, params, executor)
+    pass_ctx = transform.PassContext.current()
+    pass_config = dict(pass_ctx.config)

Review Comment:
   Right now there is no way to determine whether or not if the current pass context is the default or not. It might be possible to add though.
   
   I is not consistent with the rest of TVM to default the opt_level to 3 in meta schedule. Everywhere else defaults to 2. This does mean that we might break some user scripts.



##########
python/tvm/meta_schedule/relay_integration.py:
##########
@@ -390,16 +353,20 @@ def compile_relay(
     from tvm import relay
 
     # pylint: enable=import-outside-toplevel
-    mod, target, params, pass_config, executor = _normalize_params(
-        mod, target, params, pass_config, executor
-    )
+    mod, target, params, executor = _normalize_params(mod, target, params, executor)
+    pass_ctx = transform.PassContext.current()
+    pass_config = dict(pass_ctx.config)

Review Comment:
   Right now there is no way to determine whether or not if the current pass context is the default or not. It might be possible to add though.
   
   It is not consistent with the rest of TVM to default the opt_level to 3 in meta schedule. Everywhere else defaults to 2. This does mean that we might break some user scripts.



-- 
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] tkonolige commented on pull request #13470: [MetaSchedule] Use current pass context in compile_relay, extract_tasks

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

   @junrushao @masahi any thoughts on this? The change is from
   
   ```
   ms.compile_relay(mod, opt_level=3)
   ```
   
   to
   
   ```
   with transform.PassContext(opt_level=3):
     ms.compile_relay(mod)
   ```
   
   Although more verbose this mirrors the compilation process for non-metascheduled relay:
   
   ```
   with transform.PassContext(opt_level=3):
     vm.compile(mod)
   ```


-- 
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] tkonolige commented on pull request #13470: [MetaSchedule] Use current pass context in compile_relay, extract_tasks

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

   @junrushao @masahi This is a change to the API for compiling relay with metaschedule tuning. I believe it to be an improvement as it keeps the same passcontext api that is used in the rest of the codebase.


-- 
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