-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Avoid inserting Convert operations for irregular ov::Result case #33402
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
Conversation
dmatveev
left a comment
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.
A very promising start!
src/plugins/intel_npu/src/plugin/npuw/partitioning/online/snapshot.hpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_npu/src/plugin/npuw/partitioning/online/snapshot.cpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_npu/src/plugin/npuw/partitioning/online/snapshot.cpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_npu/src/plugin/npuw/partitioning/online/snapshot.cpp
Outdated
Show resolved
Hide resolved
f472bd5 to
543da0d
Compare
nikita-kud
left a comment
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.
| } | ||
|
|
||
| auto firstGroup = *(gset.begin()); | ||
| for(auto output_layer: firstGroup->getOutputs()) { |
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 why I added Group::getOutputs method
|
|
||
| auto matches = m_layer_matches.at(reptag->id()); | ||
|
|
||
| if(gset.size() <= 1) { |
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.
Is it even possible to get empty or just one element set for the repeating tag?
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.
In theory, yes, a single-element set is possible. E.g. the algorithm has found 32 repeating blocks A and 31 repeating blocks B, A+B merge would result in 31 AB blocks + 1 A leftover.
Those blocks are clean up later in the repeating blocks pipeline but may retain their reptags in the process (so it really depends on when this method is called).
| return lrs.count(this_layer_name) > 0; | ||
| }); | ||
|
|
||
| NPUW_ASSERT(layer_bank_iter != matches.end()); |
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.
So I assume we will have at least one match if we have more than one group.
dmatveev
left a comment
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.
Well done, LGTM! Hope it still works!
|
|
||
| auto matches = m_layer_matches.at(reptag->id()); | ||
|
|
||
| if(gset.size() <= 1) { |
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.
In theory, yes, a single-element set is possible. E.g. the algorithm has found 32 repeating blocks A and 31 repeating blocks B, A+B merge would result in 31 AB blocks + 1 A leftover.
Those blocks are clean up later in the repeating blocks pipeline but may retain their reptags in the process (so it really depends on when this method is called).
543da0d to
cc1ac68
Compare
51c805e to
05f47b7
Compare
|
|
||
| pugi::xml_node node = doc.append_child("ensemble"); | ||
| node.append_attribute("gflops") = std::to_string(ens.gflops).data(); | ||
| node.append_attribute("irregular_results") = std::to_string(ens.irregular_results).data(); |
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.
It looks like there are no tests for printing/parsing this structure, please let me know if I missed something
|
|
||
| #include "model_generator.hpp" | ||
|
|
||
| #include "openvino/op/ops.hpp" |
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.
changes by clang format
05f47b7 to
12c16e2
Compare
Details:
Essentially the partitioning ignores
::intel_npu::NPUW_F16ICoption for the irregularov::Resultconsumer.Tickets: