-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[TVMC] Allow selecting a subset of tasks to be used in tvmc tune
#12525
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
Conversation
d249a35 to
7085107
Compare
areusch
left a comment
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:
|
areusch
left a comment
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
lhutton1
left a comment
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 :)
| 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. |
lhutton1
left a comment
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
--tasksflag to thetvmc tunecommand to filter the lists of tasks to be tuned. See examples below.Motivation
Examples
--task listto show which tasks are available for tuningTests
I added a basic unit test for the
filter_tasksutility 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