You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/04/13 05:41:46 UTC
[GitHub] [tvm] zxybazh commented on a diff in pull request #10975: [Metaschedule] Make custom schedule_rule registration optional
zxybazh commented on code in PR #10975:
URL: https://github.com/apache/tvm/pull/10975#discussion_r849092781
##########
src/meta_schedule/space_generator/post_order_apply.cc:
##########
@@ -136,19 +136,31 @@ class PostOrderApplyNode : public SpaceGeneratorNode {
stack.emplace_back(sch, blocks);
continue;
}
+
Optional<String> ann = tir::GetAnn<String>(sch->GetSRef(block_rv), "schedule_rule");
- if (ann.defined() == sch_rule.defined() || (ann.defined() && ann.value() == "None")) {
+ bool has_schedule_rule = ann.defined() && runtime::Registry::Get(ann.value()) != nullptr;
Review Comment:
Shall we store the value of `runtime::Registry::Get(ann.value())` so that we don't need to do that again later?
##########
src/meta_schedule/space_generator/post_order_apply.cc:
##########
@@ -136,19 +136,31 @@ class PostOrderApplyNode : public SpaceGeneratorNode {
stack.emplace_back(sch, blocks);
continue;
}
+
Optional<String> ann = tir::GetAnn<String>(sch->GetSRef(block_rv), "schedule_rule");
- if (ann.defined() == sch_rule.defined() || (ann.defined() && ann.value() == "None")) {
+ bool has_schedule_rule = ann.defined() && runtime::Registry::Get(ann.value()) != nullptr;
+
+ if (ann.defined() && !has_schedule_rule) {
+ LOG(WARNING) << "Custom schedule rule not found, ignoring schedule_rule annotation: "
+ << ann.value();
+ }
+
+ if ((has_schedule_rule && sch_rule.defined()) ||
+ (!has_schedule_rule && !sch_rule.defined()) ||
Review Comment:
Looks good to me.
##########
tests/python/unittest/test_meta_schedule_post_order_apply.py:
##########
@@ -371,8 +371,8 @@ def test_meta_schedule_custom_search_space():
)
post_order_apply = PostOrderApply()
post_order_apply.initialize_with_tune_context(context)
- with pytest.raises(ValueError, match="Custom schedule rule not found"):
- post_order_apply.generate_design_space(mod)
+
+ post_order_apply.generate_design_space(mod)
Review Comment:
Would you please keep another test for missing custom schedule rule scenario?
##########
src/meta_schedule/space_generator/post_order_apply.cc:
##########
@@ -136,19 +136,31 @@ class PostOrderApplyNode : public SpaceGeneratorNode {
stack.emplace_back(sch, blocks);
continue;
}
+
Optional<String> ann = tir::GetAnn<String>(sch->GetSRef(block_rv), "schedule_rule");
- if (ann.defined() == sch_rule.defined() || (ann.defined() && ann.value() == "None")) {
+ bool has_schedule_rule = ann.defined() && runtime::Registry::Get(ann.value()) != nullptr;
+
+ if (ann.defined() && !has_schedule_rule) {
+ LOG(WARNING) << "Custom schedule rule not found, ignoring schedule_rule annotation: "
+ << ann.value();
+ }
+
+ if ((has_schedule_rule && sch_rule.defined()) ||
+ (!has_schedule_rule && !sch_rule.defined()) ||
+ (ann.defined() && ann.value() == "None")) {
stack.emplace_back(sch, blocks);
continue;
}
+
Array<tir::Schedule> applied{nullptr};
if (sch_rule.defined()) {
applied = sch_rule.value()->Apply(sch, /*block=*/block_rv);
- } else {
+ } else if (has_schedule_rule) {
Review Comment:
This acutally definitely hold true because we have the check in line 149.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org