graph: utils: pm: correctly handle empty optional subgraph and multi-consumer one #3282
Conversation
ShanSimu
force-pushed
the
shaojiec/fix_causal_mask
branch
from
b897dbd to
cf1adcc
Compare
| if (rep_node->get_body()->get_inner_consumer(0)->size() | ||
| == 1) | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Can you please elaborate the code above? Especially what is the for and if functions doing
There was a problem hiding this comment.
The for loop is used to iterate over all sorted_consumers and check whether their corresponding ops exist in updated_op_map_. If any op hasn't been processed (i.e., all_ops_in_map == false), it further checks whether the out_node (output node) is a multi-consumer. If it's not, the subsequent processing is skipped using continue.
There was a problem hiding this comment.
For the for loop, it seems redundant as the outer loop (L368) already iterates all the consumers, can you just consider that specific consumer?
And when all_ops_in_map = true, why do we still need to match them again in L411? If I understand correctly, in the map means it's already matched?
There was a problem hiding this comment.
Thanks for commenting. I have removed for loop and just consider the specific op.
Add an else branch to set consumer_matched to true and then break.
| continue; | ||
| if (rep_node->get_body()->get_inner_consumer(0)->size() == 1) | ||
| continue; | ||
| // For repetition case, check if multi consumers have been matched |
There was a problem hiding this comment.
Need a UT to verify the code here?
ShanSimu
force-pushed
the
shaojiec/fix_causal_mask
branch
from
cf1adcc to
5def59f
Compare
ShanSimu
force-pushed
the
shaojiec/fix_causal_mask
branch
2 times, most recently
from
6178727 to
94af97e
Compare
|
make test |
ShanSimu
force-pushed
the
shaojiec/fix_causal_mask
branch
from
94af97e to
f3ecaea
Compare
|
make test |
ShanSimu
force-pushed
the
shaojiec/fix_causal_mask
branch
from
f3ecaea to
36fa6a6
Compare
ShanSimu
force-pushed
the
shaojiec/fix_causal_mask
branch
from
36fa6a6 to
51f7fb2
Compare
|
make test |
Description
During pattern matching, we found a bug that occurs when two optional subgraphs are connected: if the first one is empty and the second one's input has multi-consumers, it leads to incorrect behavior. This PR is intended to fix that bug.
Fixes MFDNN-13660