Skip to content

EliminatePad and PadFusion support Pad12 positive indexes#18278

Merged
itikhono merged 10 commits intoopenvinotoolkit:masterfrom
evkotov:pad12_pad_fusion
Jul 5, 2023
Merged

EliminatePad and PadFusion support Pad12 positive indexes#18278
itikhono merged 10 commits intoopenvinotoolkit:masterfrom
evkotov:pad12_pad_fusion

Conversation

@evkotov
Copy link
Copy Markdown
Contributor

@evkotov evkotov commented Jun 28, 2023 •
edited
Loading

Details:

  • fix EliminatePad transformation to handle Pad12 positive indexes
  • fix PadFusion group of transformations (PadFusionAvgPool, PadFusionConvolution, PadFusionConvolutionBackpropData,
    PadFusionGroupConvolution, PadFusionGroupConvolutionBackpropData) Pad12 positive indexes
  • there are checks in such a transformations not to handle Pad with negative indexes
  • rewrite unit tests to check both old and new Pad operations
  • add unit test PadElimination with negative indexes

Tickets:

  • 112279

@evkotov evkotov added the category: transformations OpenVINO Runtime library - Transformations label Jun 28, 2023
@evkotov evkotov added this to the 2023.1 milestone Jun 28, 2023
@evkotov evkotov requested a review from a team as a code owner June 28, 2023 17:31
@evkotov evkotov self-assigned this Jun 28, 2023
itikhono
itikhono reviewed Jul 3, 2023
View reviewed changes
src/common/transformations/src/transformations/common_optimizations/pad_fusion.cpp
auto pad_node_pattern =
pattern::wrap_type<opset5::Pad>({data_pattern, pads_begin_pattern, pads_end_pattern, pad_value_pattern},
pattern::consumers_count(1));
pattern::wrap_type<op::util::PadBase>({data_pattern, pads_begin_pattern, pads_end_pattern, pad_value_pattern},
Copy link
Copy Markdown
Contributor

@itikhono itikhono Jul 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a note, no changes from your side required: I hope the check on line 58 (pad_fusion.cpp) should already detect the negative pad values

Copy link
Copy Markdown
Contributor Author

@evkotov evkotov Jul 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I noticed that before PR creation

@evkotov evkotov requested a review from t-jankowski July 4, 2023 11:09
@itikhono itikhono enabled auto-merge (squash) July 5, 2023 09:30
@itikhono itikhono merged commit f313dde into openvinotoolkit:master Jul 5, 2023
@evkotov evkotov deleted the pad12_pad_fusion branch July 5, 2023 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mateusztabaka mateusztabaka Awaiting requested review from mateusztabaka

@t-jankowski t-jankowski Awaiting requested review from t-jankowski

1 more reviewer

@itikhono itikhono itikhono approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@evkotov evkotov

Labels

category: transformations OpenVINO Runtime library - Transformations

Projects

None yet

Milestone

2023.1

Development

Successfully merging this pull request may close these issues.

2 participants

@evkotov @itikhono