Skip to content

Convolution#3922

Merged
ilyachur merged 29 commits intoopenvinotoolkit:masterfrom
jdanieck:convolution
Feb 2, 2021
Merged

Convolution#3922
ilyachur merged 29 commits intoopenvinotoolkit:masterfrom
jdanieck:convolution

Conversation

@jdanieck
Copy link
Copy Markdown
Contributor

@jdanieck jdanieck commented Jan 19, 2021

Convolution reference implementation refactored. Ticket 37429.

The easiest way to understand implementation is to start with 1D Convolution, then 2D and 3D. If you are not familiar with Convolution concepts (channels, strides, padding, dilation) I suggest to read articles attached to the ticket first.

Implementation details:

  • ConvolutionBackpropData reference implementation was moved to separate file - no changes in the implementation.
  • Removed CoordinateTransform usage to improve performance (details in the ticket) - indexes are calculated explicitly.
  • 1D, 2D, 3D convolution is expressed in code explicitly to improve readability (educational use case) and at the same time improve performance.
  • SLT tests extended by 1-D Convolution tests
  • Serialization SLT added
  • Specification refactored

@jdanieck jdanieck marked this pull request as ready for review January 21, 2021 08:40
@jdanieck jdanieck requested a review from a team as a code owner January 21, 2021 08:40
@ggalieroc-zz ggalieroc-zz self-requested a review January 22, 2021 08:50
Copy link
Copy Markdown

@elszkowski elszkowski left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@jdanieck
Copy link
Copy Markdown
Contributor Author

jdanieck commented Feb 2, 2021

@ilyachur ready to merge.

@ilyachur ilyachur merged commit c1b0b03 into openvinotoolkit:master Feb 2, 2021
@ilyachur
Copy link
Copy Markdown
Contributor

ilyachur commented Feb 2, 2021

Merged

@jdanieck jdanieck deleted the convolution branch February 2, 2021 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants