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] Autotuning - Hardware configs not default #7792
Conversation
Just to add a little more context, the reason for this change is that we recently had someone try out TVMC and get very confused why it was producing different tuning results than autoscheduling the traditional way. It turns out it was due to |
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 for the PR. I think it just needs some clarification for the benefit of the end-user.
python/tvm/driver/tvmc/autotuner.py
Outdated
|
||
auto_scheduler_group = parser.add_argument( | ||
"--enable-hardware-params", | ||
help="enable hardware specific controls such as cores, threads, memory etc.", |
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.
Can you mention what is the default?
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.
This argument name and description are unclear to me:
- When disable, what happen?
- When enable, how do I "control" hardware specific factors?
- The action (store false) is also non-intuitive.
The semantic now becomes when we have --enable-hardware-params
, the rest hardware parameter arguments are ignored. This doesn't make any sense to me. I guess the root cause of the confusion is why --enable-XX
stores false.
Since your motivation of this PR is making sure when users don't want to care about these parameters, the purpose of this PR is to make the default hardware parameters more reasonable, and the way it achieves this goal is via auto-scheduler's default values, which may vary on different platforms.
If the above analysis is correct, then the problem becomes "auto-scheduler already has a good mechanism to determine the default hardware parameters, do we still need another set of default values in TVMC"? If the answer is no, then we could simply aggregate all hardware parameters to a JSON file. In other words, removing the arguments such as --num-cores
and add an argument --hardware-params <JSON file>
. When this file is not specified, we pass None
to auto-scheduler so that the default values will be used. IMHO, only advance users will use these hardware parameters, so it should be fine to let them prepare a JSON file for it. We just need to document all available parameters somewhere, or simply point to the auto-scheduler tutorial to make sure advance users can find the resource when needed.
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.
Sorry! I should have used store_true. I think that made everything confusing. My goal was to use the default auto_schedule parameters if the user didn't specify anything. Hardware parameters can still be specified in the command line as in the main branch.
Perhaps we can push this PR where advanced users can still use command line to provide specific hardware arguments, and consider doing the JSON extension later?
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.
That makes a lot sense. However, IMHO, this still confuses people, unless you could make those commandline arguments visible only when --enable-hardware-params
is specified (like put them to a subparser), so that people won't see those arguments when tvmc ... -h
but can only see them with tvmc ... --enable-hardware-params -h
. Otherwise I could imagine an advance user specifies --num-cores
but feels weird about why it isn't effective, and finally finds that --enable-hardware-params
is required.
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.
Adding a hardware-params subparser is an excellent idea.
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.
Adding a hardware-params subparser is an excellent idea.
Yes, I think a subparser would be the best option in this case. Also making sure the working on the help option is clear.
This PR adds an argument to the parser which sets on default, the hardware configurations to be off. Now experienced users can still toggle with hardware settings but the novice user will experience the same settings as using tvm in another context.