Skip to content
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

AutoTVM: selecting tuning templates when extracting task #4338

Merged
merged 4 commits into from Nov 16, 2019
Merged

AutoTVM: selecting tuning templates when extracting task #4338

merged 4 commits into from Nov 16, 2019

Conversation

zhenhuaw-me
Copy link
Contributor

Make the procedure of trying new templates easier.

Would you please have a look at this @eqy @merrymercy @tmoreau89 @vinx13

Make the procedure of trying new templates easier.

Test: tests/python/relay/test_autotvm_task_extraction.py
Copy link
Contributor

@comaniac comaniac left a 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. This new optional argument indicates the template key when creating tasks, but what happens if I have more than one kinds of ops (e.g., dense, conv2d) in a model with different template keys? Specifically, in the following model:

conv2d
conv2d
dense

If I put template_key='winograd', then the semantic to dense is incorrect. On the other hand, I may only want to the first conv2d to use Winograd but keep the second one direct.

tests/python/relay/test_autotvm_task_extraction.py Outdated Show resolved Hide resolved
@zhenhuaw-me
Copy link
Contributor Author

Thank you for the review @comaniac @tmoreau89 @vinx13 , really good points to have a dict holding the op to key mapping! I will update correspondingly.

@zhenhuaw-me
Copy link
Contributor Author

zhenhuaw-me commented Nov 15, 2019

Comments addressed, please have a look into this when you are available :) @comaniac @tmoreau89 @vinx13

For the template key dict part, now have a TOPI op to template key mapping. Relay ops may have many topi ops which are implemented with different template keys, so maybe not good enough to be the dict keys.

For the complicate case @comaniac mentioned in #4338 (review) , I guess it could be the user to handle as it is hard to generalize and the API (if there were such) could be annoying.

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you that the case uses different template keys for one kind of ops could be too complicate to be handled here so let's put it away first.

python/tvm/autotvm/task/relay_integration.py Outdated Show resolved Hide resolved
python/tvm/autotvm/task/relay_integration.py Outdated Show resolved Hide resolved
python/tvm/autotvm/task/relay_integration.py Outdated Show resolved Hide resolved
@zhenhuaw-me
Copy link
Contributor Author

Thank you for the quick look @comaniac ! Now updated.

@comaniac
Copy link
Contributor

Thanks. LGTM.
btw, would you update the tutorial as well?

@zhenhuaw-me
Copy link
Contributor Author

Thank you @comaniac ! I will update the tutorials in another PR.

@tmoreau89 @vinx13 Would you like to have a look please :)

@vinx13 vinx13 merged commit ccde31f into apache:master Nov 16, 2019
@vinx13
Copy link
Member

vinx13 commented Nov 16, 2019

Thanks @jackwish @comaniac @tmoreau89 this is merged

@zhenhuaw-me zhenhuaw-me deleted the autotvm/template-key branch November 18, 2019 01:51
zxy844288792 pushed a commit to zxy844288792/tvm that referenced this pull request Nov 26, 2019
* AutoTVM: selecting tuning templates when extracting task

Make the procedure of trying new templates easier.

Test: tests/python/relay/test_autotvm_task_extraction.py

* Use dict to match key for topi ops

* fix lint issue

* be more pythonic :)
yongwww pushed a commit to neo-ai/tvm that referenced this pull request Nov 26, 2019
* AutoTVM: selecting tuning templates when extracting task

Make the procedure of trying new templates easier.

Test: tests/python/relay/test_autotvm_task_extraction.py

* Use dict to match key for topi ops

* fix lint issue

* be more pythonic :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants