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

[AutoScheduler] Bug fix & Custom sketch support #7260

Merged
merged 13 commits into from
Jan 19, 2021

Conversation

jcf94
Copy link
Contributor

@jcf94 jcf94 commented Jan 12, 2021

cc @comaniac @merrymercy

python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
src/auto_scheduler/search_policy/sketch_policy_rules.cc Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved

CONDITION_NUM = {"pass": 0, "apply": 1, "apply_and_skip_rest": 2}

def __init__(self, meet_condition_func, apply_func, rule_name="CustomSketchRule"):
Copy link
Contributor

Choose a reason for hiding this comment

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

From your test case I think we should not provide a default rule name. Otherwise it's easy to get the rule name conflict if users call PreloadCustomSketchRule twice.

python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
@jcf94 jcf94 requested a review from comaniac January 18, 2021 02:27
@comaniac
Copy link
Contributor

I'm still not clear about the registration mechanism with the decorator. I agree with you that using the preload callback should be sufficient for users to specify custom rules, and it seems just a fast path without any additional functions that the preload callback doesn't have. Accordingly, should we just remove the decorator and registry to make the design simple and easy to maintain?

@jcf94
Copy link
Contributor Author

jcf94 commented Jan 18, 2021

I'm still not clear about the registration mechanism with the decorator. I agree with you that using the preload callback should be sufficient for users to specify custom rules, and it seems just a fast path without any additional functions that the preload callback doesn't have. Accordingly, should we just remove the decorator and registry to make the design simple and easy to maintain?

I agree, decorator has been removed.

@jcf94 jcf94 requested a review from merrymercy January 19, 2021 00:46
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.

LGTM. Thanks.

@comaniac comaniac merged commit 5d95105 into apache:main Jan 19, 2021
@comaniac
Copy link
Contributor

Thanks @jcf94 @merrymercy

@jcf94 jcf94 deleted the custom_sketch branch January 19, 2021 06:24
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Jan 20, 2021
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jan 21, 2021
electriclilies pushed a commit to electriclilies/tvm that referenced this pull request Feb 18, 2021
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.

4 participants