-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TVMC] Allow selecting a subset of tasks to be used in tvmc tune
#12525
Conversation
d249a35
to
7085107
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @PhilippvK, this looks like a great addition! left a couple comments here and there
@PhilippvK i think this one needs a rebase |
e4d85de
to
c1306f7
Compare
Short update:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @PhilippvK, could you address the lint issue?
As I have to retrigger the CI anyway so I want to propose another small addition: For AutoTVM, in addition to the task description, I could also print Example:
Do you think this would be a good addition to |
@areusch Can I have an update on this? |
@tvm-bot rerun |
sorry @PhilippvK I missed this one. feel free to ping me on discord if that happens again. I retriggered the CI, I'm good for @leandron to merge this if it passes.
Sure that seems like a good addition! |
c4f6894
to
1e7d121
Compare
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 by @-ing them in a comment. Generated by tvm-bot |
Sorry @PhilippvK. It took me a while to catch up with this. Can you rebase and then we move to get this merged? |
c66a3e6
to
6c9c16b
Compare
6c9c16b
to
ff33ca0
Compare
ff33ca0
to
631f97d
Compare
@leandron Rebased on |
@tvm-bot rerun |
631f97d
to
f11bb22
Compare
f11bb22
to
1aff401
Compare
Rebased to fix conflict + I added 2 small AutoTVM related improvements:
|
[TVMC] Address PR comments [TVMC] Properly print task names of for AutoScheduler [TVMC] Implement print_task_list to cleanup code
[Tests] add missing tvm.testing.main call to test_autotuner.py
1aff401
to
858a722
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed this PR and think it looks very helpful, thanks! I had a couple of nits around the testing, but given this PR has been rebased a few times I'm happy for them to be fixed in a follow-up, let me know :)
@@ -207,3 +209,27 @@ def test_autotune_pass_context(mock_pc, onnx_mnist, tmpdir_factory): | |||
# AutoTVM overrides the pass context later in the pipeline to disable AlterOpLayout | |||
assert mock_pc.call_count == 2 | |||
assert mock_pc.call_args_list[0][1]["opt_level"] == 3 | |||
|
|||
|
|||
def test_filter_tasks_valid(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: it would be helpful to parameterize these tests similar to below, so failures are reported separately in the CI log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops I forgot to address this. Can we merge this anyway?
@tvm-bot rerun |
@lhutton1 Hey, I implemented the proposed changes and the CI passed. Feel free to merge after quickly having a look at the refactored unit tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @PhilippvK, LGTM!
Thanks @PhilippvK @areusch @leandron! |
This adds a
--tasks
flag to thetvmc tune
command to filter the lists of tasks to be tuned. See examples below.Motivation
Examples
--task list
to show which tasks are available for tuningTests
I added a basic unit test for the
filter_tasks
utility intests/python/driver/tvmc/test_autotuner.py
.Open Questions
While the (truncated) string representations of AutoTVM tasks are quite helpful to pick the correct tasks, using AutoScheduler the tasks can not really be distinguished from each other (only by index). Is there a way to get similar information from AutoScheduler tasks?cc @Mousius @gromero