Skip to content
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

[PyOV] Extend Python API with Col2Im-15 #24569

Merged
merged 10 commits into from
May 23, 2024

Conversation

p-wysocki
Copy link
Contributor

Copy link

@jiwaszki jiwaszki left a comment

Choose a reason for hiding this comment

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

Overall LGTM.

Comment on lines 68 to 73
for param in [strides, dilations]:
if param is None:
param = [1, 1]
for param in [pads_begin, pads_end]:
if param is None:
param = [0, 0]

Choose a reason for hiding this comment

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

Can you test these None cases somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are tested in the first test case: ((3, 4), [2, 2], [1, 1], [3, 2, 2], []),. The last empty list is params, which are given into the op constructor as *params.

@@ -6,4 +6,5 @@

# TODO (ticket 138273): Add previous opset operators at the end of opset15 development
from openvino.runtime.opset1.ops import parameter
from openvino.runtime.opset15.ops import col2im
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need from openvino.runtime.opset1.ops import parameter above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not certain, my guess is that it was needed in opset15 for now for the tests to use it. One way or another since this PR is targeted for Code Freeze let's come back to it another time.

@mlukasze mlukasze enabled auto-merge May 20, 2024 05:24
@p-wysocki p-wysocki added this to the 2024.2 milestone May 20, 2024
github-merge-queue bot pushed a commit that referenced this pull request May 21, 2024
### Details:
- Similar in functionality to
https://pytorch.org/docs/stable/generated/torch.nn.Fold.html, Col2Im is
`torch.nn.Fold` restricted to two output spatial dimensions

### Tickets:
 - CVS-138919

### Related PRs:
- #24197
- #23947
- #24569

---------

Co-authored-by: Michal Lukaszewski <[email protected]>
@mlukasze mlukasze added this pull request to the merge queue May 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 22, 2024
@mlukasze mlukasze enabled auto-merge May 23, 2024 05:31
@mlukasze mlukasze added this pull request to the merge queue May 23, 2024
Merged via the queue into openvinotoolkit:master with commit c4f33ce May 23, 2024
116 checks passed
@p-wysocki p-wysocki deleted the col2im_python branch May 23, 2024 08:59
github-merge-queue bot pushed a commit that referenced this pull request Jul 3, 2024
### Details:
 - Register `Col2im-15` in CPU Plugin
- Use reference implementation, used
#23844 as reference
 - Add tests

### Tickets:
 - CVS-142438

### Related PRs:
 - #24548
 - #24197
 - #23947
 - #24569

---------

Co-authored-by: Michal Lukaszewski <[email protected]>
Co-authored-by: Maksim Kutakov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Python API OpenVINO Python bindings Code Freeze
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants