[microNPU][4] Add the cascader Proposal generator#9959
Conversation
jacobbohlin
force-pushed
the
ethosu-cascader-7
branch
from
a9819e7 to
5928c6b
Compare
mbaret
force-pushed
the
ethosu-cascader-7
branch
from
5928c6b to
097b250
Compare
The Proposal generator takes optimal Plans and combines them to find optimal 'Proposals' - sets of disjoint Plans that cover every Part in a CascaderGraph. It ultimately produces a Pareto-frontier of 'optimal' Proposals in terms of estimated cycles and memory usage. Change-Id: Id42099819a596496a5769bae22f08eeb75ec69b6
mbaret
force-pushed
the
ethosu-cascader-7
branch
from
097b250 to
d1af75a
Compare
mbaret
requested review from
areusch,
comaniac,
icemelon,
jroesch,
junrushao,
merrymercy,
tqchen,
yzhliu and
zhiics
as code owners
mbaret
force-pushed
the
ethosu-cascader-7
branch
from
d1af75a to
a272e8a
Compare
manupak
left a comment
manupak
left a comment
There was a problem hiding this comment.
Thanks @mbaret .
I have let some comments; some questions, some for more docs and some for slight refactors.
Happy to take these in a follow up to allow reviews of other PRs stacked on top of this.
I think we should try to progress this PR.
| std::sort(proposals.begin(), proposals.end(), [](const Proposal& a, const Proposal& b) -> bool { | ||
| return a->GetMemoryUsage() < b->GetMemoryUsage(); | ||
| }); | ||
| std::vector<std::array<float, 2>> costs; |
There was a problem hiding this comment.
for future : it might be better to use a typed struct here
| namespace ethosu { | ||
| namespace cascader { | ||
|
|
||
| std::unordered_set<TensorConfig> GetPlanBoundaryConfigs(const Plan& plan) { |
There was a problem hiding this comment.
Should we have docs for this function ?
| return boundary_configs; | ||
| } | ||
|
|
||
| bool IsPlanCompatible(const Proposal& proposal, const std::vector<Part>& plan_part_group, |
There was a problem hiding this comment.
Should we have docs for this function ?
| return true; | ||
| } | ||
|
|
||
| std::unordered_map<Part, std::vector<Plan>, ObjectPtrHash, ObjectPtrEqual> CreatePlansByPart( |
There was a problem hiding this comment.
Should we have docs for this function ?
| return plans_by_part; | ||
| } | ||
|
|
||
| Proposal AddPlanToProposal(const Proposal& proposal, const Plan& plan, |
There was a problem hiding this comment.
Should we have docs for this functions ?
| return std::find(plan->GetPartGroup().begin(), plan->GetPartGroup().end(), | ||
| value) == plan->GetPartGroup().end(); | ||
| }); | ||
| // std::sort(residual_proposal_group.begin(), residual_proposal_group.end()); |
| const CascaderGraph& graph, const HomeMap& home_map, const CascaderOptions options, | ||
| const std::unordered_map<Part, std::vector<Plan>, ObjectPtrHash, ObjectPtrEqual>& plans_by_part, | ||
| const std::vector<Part>& partial_proposal_group, | ||
| std::unordered_map<std::vector<Part>, std::vector<Proposal>>* proposals_by_group) { |
There was a problem hiding this comment.
Just curious, what is the reasoning to use a pointer as opposed to references elsewhere ?
| value) == plan->GetPartGroup().end(); | ||
| }); | ||
| // std::sort(residual_proposal_group.begin(), residual_proposal_group.end()); | ||
| const auto& residual_proposals = GeneratePartialProposals( |
There was a problem hiding this comment.
A comment would help here to differentiate between "residual_proposal_group" vs "residual_proposals"
| value) == plan->GetPartGroup().end(); | ||
| }); | ||
| // std::sort(residual_proposal_group.begin(), residual_proposal_group.end()); | ||
| const auto& residual_proposals = GeneratePartialProposals( |
There was a problem hiding this comment.
It also feels like constant context data is passed onto the recursive call that does not change between calls.
Should we restructure this to be a class to hold such data (e.g. graph, home_map, proposals_by_group) ?
| std::unordered_map<Tensor, std::vector<MemoryRegion>, ObjectPtrHash, ObjectPtrEqual> | ||
| mhome_map; | ||
| for (const auto& it : home_map) { | ||
| std::vector<MemoryRegion> home_regions; |
There was a problem hiding this comment.
Did not we have a utility to convert Arrays to Vectors somewhere ?
|
We can unblock the reviews of the next PR, if we could address the comments of this PR in a seperate follow up PR. |
|
Yup, happy to take it as a follow up. |
manupak
left a comment
manupak
left a comment
There was a problem hiding this comment.
LGTM! (bar the comments that is deffered to the follow up)
|
Thanks! @mbaret |
* [microNPU][4] Add the cascader Proposal generator The Proposal generator takes optimal Plans and combines them to find optimal 'Proposals' - sets of disjoint Plans that cover every Part in a CascaderGraph. It ultimately produces a Pareto-frontier of 'optimal' Proposals in terms of estimated cycles and memory usage. Change-Id: Id42099819a596496a5769bae22f08eeb75ec69b6 * Fixes Change-Id: I4f5f2a298bd3bb379c7c8d179150358923b0dd66
2 participants
RFC: apache/tvm-rfcs#37
Issue: #9429
The Proposal generator takes optimal Plans and combines them to find optimal 'Proposals' - sets of disjoint Plans that cover every Part in a CascaderGraph. It ultimately produces a Pareto-frontier of 'optimal' Proposals in terms of estimated cycles and memory usage.