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 2020/12/21 22:14:50 UTC

[GitHub] [tvm] comaniac opened a new pull request #7143: [AutoScheduler] Python based measure callbacks

comaniac opened a new pull request #7143:
URL: https://github.com/apache/tvm/pull/7143


   The current auto_scheduler callbacks can only be implemented in C++. This PR exposes the interface of measure callbacks to Python so that people can plug in their own callback functions easily.
   
   Note: looks like I cannot pass the `SearchPolicy` node to the `PackedFunc`. Specifically, `callback_func(policy, inputs, results);` results in the type mismatching error. This also prevents us from introducing the Python API for `SearchCallback`. Any advise is appreciated.
   
   cc @merrymercy @jcf94 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] comaniac commented on pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#issuecomment-750418614


   I also took a look at the `SearchCallback` (it should be named to something like `PreSearchCallback` because it will be called once in the beginning of search instead of every round) but decided not to add a Python interface in this PR. Since we don't expose useful SearchPolicy APIs to Python, the Python-based callback can do nothing at this moment. Meanwhile, it's worthwhile to consider `PreloadCustomSketchRule` as @antinucleon suggested in another PR.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] jcf94 edited a comment on pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
jcf94 edited a comment on pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#issuecomment-750704577


   > > Actually we already have a `init_search_callbacks` in the constructor function of SearchPolicy which will be called before the search process. And we also have the `PreloadCustomSketchRule` in our dev repo ...
   > 
   > Yeah that's why I bring up this, but my point is these callbacks are C-based instead of Python so there are not that convenient for users to experiment.
   
   Oh, I find that comment: https://github.com/apache/tvm/pull/7132#discussion_r547936669
   I agree with your point.
   So seems we will need to:
   1. Add the `PreloadCustomSketchRule` which enables users to add their own custom sketches. (And that's what we have already planed long before but didn't take action to bring that here from our dev repo ....)
   2. Just like your have added a python based `MeasureCallback` here, we'd better add a python based `SearchCallback` for init_search_callbacks.
   3. Better re-arrange the `rules->push_back(.......)` part, maybe also move them to Python as a `InitSearchCallbak`.
   
   Also cc @merrymercy @antinucleon 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] merrymercy commented on a change in pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#discussion_r548440024



##########
File path: tests/python/relay/test_auto_scheduler_tuning.py
##########
@@ -23,6 +23,15 @@
 from test_auto_scheduler_task_extraction import get_network
 
 
+class CustomMeasureCallback(auto_scheduler.measure.PythonBasedMeasureCallback):
+    """A simple Python-based callback for testing."""
+
+    def callback(self, policy, inputs, results):
+        assert isinstance(policy, auto_scheduler.search_policy.SketchPolicy)
+        for inp, res in zip(inputs, results):
+            print(policy, inp, res)
+
+

Review comment:
       ```suggestion
   ```
   Delete useless old code




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] merrymercy commented on a change in pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#discussion_r547991726



##########
File path: tests/python/relay/test_auto_scheduler_tuning.py
##########
@@ -23,6 +23,15 @@
 from test_auto_scheduler_task_extraction import get_network
 
 
+class CustomMeasureCallback(auto_scheduler.measure.PythonBasedMeasureCallback):

Review comment:
       It is better to move this test to here (https://github.com/apache/tvm/blob/e51bcdd1ed99fc813454c68911f8032c852f48b4/tests/python/unittest/test_auto_scheduler_search_policy.py#L71).
   
   Because this measure callback is not related to relay, we should not put it under `tests/python/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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] jcf94 commented on pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
jcf94 commented on pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#issuecomment-750699810


   > I also took a look at the `SearchCallback` (it should be named to something like `PreSearchCallback` because it will be called once in the beginning of search instead of every round) but decided not to add a Python interface in this PR. Since we don't expose useful SearchPolicy APIs to Python, the Python-based callback can do nothing at this moment. Meanwhile, it's worthwhile to consider `PreloadCustomSketchRule` as @antinucleon suggested in another PR.
   
   Actually we already have a `init_search_callbacks` in the constructor function of SearchPolicy which will be called before the search process. And we also have the `PreloadCustomSketchRule` in our dev repo ...


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] comaniac commented on pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#issuecomment-750700286


   > Actually we already have a `init_search_callbacks` in the constructor function of SearchPolicy which will be called before the search process. And we also have the `PreloadCustomSketchRule` in our dev repo ...
   
   Yeah that's why I bring up this, but my point is these callbacks are C-based instead of Python so there are not that convenient for users to experiment.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] merrymercy commented on a change in pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#discussion_r547991726



##########
File path: tests/python/relay/test_auto_scheduler_tuning.py
##########
@@ -23,6 +23,15 @@
 from test_auto_scheduler_task_extraction import get_network
 
 
+class CustomMeasureCallback(auto_scheduler.measure.PythonBasedMeasureCallback):

Review comment:
       It is better to move this test to here (https://github.com/apache/tvm/blob/e51bcdd1ed99fc813454c68911f8032c852f48b4/tests/python/unittest/test_auto_scheduler_search_policy.py#L71).
   
   Because this measure callback is not related to relay. We should not put it under `tests/python/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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] comaniac commented on pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#issuecomment-749962826


   Per offline discussion, we cast the abstract SearchPolicy to the actual instance so that we can pass it to the packed function.
   
   @merrymercy @jcf94 PTAL.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] jcf94 commented on pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
jcf94 commented on pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#issuecomment-750704577


   > > Actually we already have a `init_search_callbacks` in the constructor function of SearchPolicy which will be called before the search process. And we also have the `PreloadCustomSketchRule` in our dev repo ...
   > 
   > Yeah that's why I bring up this, but my point is these callbacks are C-based instead of Python so there are not that convenient for users to experiment.
   
   Oh, I find that comment: https://github.com/apache/tvm/pull/7132#discussion_r547936669
   I agree with your point.
   So seems we will need to:
   1. Add the `PreloadCustomSketchRule` which enables users to add their own custom sketches. (And that's what we have already planed before but didn't take action to bring that here from our dev repo ....)
   2. Just like your have added a python based `MeasureCallback` here, we'd better add a python based `SearchCallback` for init_search_callbacks.
   3. Better re-arrange the `rules->push_back(.......)` part, maybe also move them to Python as a `InitSearchCallbak`.
   
   Also cc @merrymercy @antinucleon 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] comaniac commented on a change in pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#discussion_r548107307



##########
File path: tests/python/relay/test_auto_scheduler_tuning.py
##########
@@ -23,6 +23,15 @@
 from test_auto_scheduler_task_extraction import get_network
 
 
+class CustomMeasureCallback(auto_scheduler.measure.PythonBasedMeasureCallback):

Review comment:
       Moved.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] merrymercy merged pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
merrymercy merged pull request #7143:
URL: https://github.com/apache/tvm/pull/7143


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] jcf94 edited a comment on pull request #7143: [AutoScheduler] Python based measure callbacks

Posted by GitBox <gi...@apache.org>.
jcf94 edited a comment on pull request #7143:
URL: https://github.com/apache/tvm/pull/7143#issuecomment-750704577


   > > Actually we already have a `init_search_callbacks` in the constructor function of SearchPolicy which will be called before the search process. And we also have the `PreloadCustomSketchRule` in our dev repo ...
   > 
   > Yeah that's why I bring up this, but my point is these callbacks are C-based instead of Python so there are not that convenient for users to experiment.
   
   Oh, I find that comment: https://github.com/apache/tvm/pull/7132#discussion_r547936669
   I agree with your point.
   So seems we will need to:
   1. Add the `PreloadCustomSketchRule` which enables users to add their own custom sketches. (And that's what we have already planed long before but didn't take action to bring that here from our dev repo ....)
   2. Just like your have added a python based `MeasureCallback` here, we'd better add a python based `SearchCallback` for init_search_callbacks.
   3. Better re-arrange the `rules->push_back(.......)` part, maybe also move them to Python as a `InitSearchCallback`.
   
   Also cc @merrymercy @antinucleon 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org