-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[MetaSchedule] Apply-History-Best Task Filtering #11692
[MetaSchedule] Apply-History-Best Task Filtering #11692
Conversation
CC: @zxybazh @csullivan |
src/meta_schedule/extracted_task.cc
Outdated
stack.pop_back(); | ||
if (tensor->op->IsInstance<PlaceholderOpNode>()) { | ||
// do nothing | ||
} else if (tensor->op->IsInstance<ComputeOpNode>()) { |
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.
} else if (tensor->op->IsInstance<ComputeOpNode>()) { | |
} else if (tensor->op->IsInstance<ComputeOpNode>() || tensor->op->IsInstance<ExternOpNode>()) { |
You'll need to check for extern ops here as well (ref).
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 is certain an interesting design tradeoff. MetaSchedule/AutoScheduler default rules are not supposed to handle all the ExternOp and HybridOp in every model, because they are already scheduled (e.g. the NMS operator), or do not need to be scheduled (e.g. external library). Therefore, we are filtering out all those operations by default to make sure MetaSchedule doesn't break on customer models.
Alternatively, given we support customized filtering methods, do you think it's a good idea if we expose another method that doesn't rule out extern-op for customized usecases?
This PR enables task filtering in Apply-History-Best, which is used in Relay/Relax integration. Previously, even though a task is ruled out during task extraction, it still shows up in Relay compilation due to the lack of filtering on `Apply-History-Best`. However, TE-to-TIR conversion `te.CreatePrimFunc` doesn't support all cases with hybrid operators involved, which leads to post-tuning failure affecting multiple models.
630c605
to
2f21055
Compare
target, | ||
params, | ||
schedule_fn, | ||
te_filter_func="meta_schedule.DefaultTaskFilterAllowExtern", |
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.
@csullivan there are a couple of options in my mind for us to unbreak MS's current workflow with customer models:
- use
te_filter_func
when we want to specifically allow somete.extern
operation; - use a different
te.extern_prim_func
to direct the default task filter to accept them by default - use annotations to instruct the default task filter instead
let me know what you think!
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.
@junrushao1994 thanks for the discussion! I think the approach you have taken with a separate filter function is a good solution for now.
From the integration side I think it could indeed be desirable to separate out the functionality into a new TE Op (your second option I believe) that can be included by the default filter func so that it is transparent to end user. But for now I think it's perfectly fine to approach it when the need arises. Thanks!
This PR enables task filtering in Apply-History-Best, which is used in
Relay/Relax integration. Previously, even though a task is ruled out
during task extraction, it still shows up in Relay compilation due to
the lack of filtering on
Apply-History-Best
. However, TE-to-TIRconversion
te.CreatePrimFunc
doesn't support all cases with hybridoperators involved, which leads to post-tuning failure affecting
multiple models.