Fix eltwise alter op layout for broadcast axis #11337
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can you add a test or more description of what problem it is solving? |
Added PR description and tests |
elvin-n
force-pushed
the
amalyshe/aol_elwise_fix
branch
from
39eb396 to
2504599
Compare
elvin-n
force-pushed
the
amalyshe/aol_elwise_fix
branch
from
2504599 to
cc3779b
Compare
masahi
approved these changes
May 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue appeared after implementation of handling broadcast axis on dependent eltwise operators in alter op layout.
The pattern that I had
conv->addand it happened that the block size was equal to the number of output channels. In one moment during network compilation we got a situation when we have already blocked layout and alter_op_layout is called one more time. During this second invocation the part dedicated for determination of the layout for second input ofadddetects1unconditionally as broadcast axis and puts wronglayout_transform(->NCHW1c). This is wrong. In our case 1 was a reminder. Need to take into account block part as well in such cases.I believe test
test_alter_layout_blocked_no_broadcastshows situation clearly. The second testtest_alter_layout_re_blocking_broadcastis not required on 100% but let's have it as well - it reproduce scenario of re-blocking of axises and verifies that our algos are correct.+@lazycal