[Py OV] Alias classes and functions in flatten openvino namespace #28085

almilosz · 2024-12-16T20:29:36Z

Details:

Add imports from _pyopenivno on the top of the openvino __init__.py
Move Node binary operators support at the bottom of openvino __init__.py

Classes and functions available in openvino/runtime will match those in flatten openvino

Tickets:

CVS-129453

Signed-off-by: Alicja Miloszewska <[email protected]>

…into almilosz/alias-classes Signed-off-by: Alicja Miloszewska <[email protected]>

Signed-off-by: Alicja Miloszewska <[email protected]>

…into almilosz/alias-classes Signed-off-by: Alicja Miloszewska <[email protected]>

Signed-off-by: Alicja Miloszewska <[email protected]>

rkazants · 2024-12-17T13:50:12Z

tools/ovc/openvino/__init__.py

+# Extend Node class to support binary operators
+Node.__add__ = opset13.add
+Node.__sub__ = opset13.subtract
+Node.__mul__ = opset13.multiply
+Node.__div__ = opset13.divide
+Node.__truediv__ = opset13.divide
+Node.__radd__ = lambda left, right: opset13.add(right, left)
+Node.__rsub__ = lambda left, right: opset13.subtract(right, left)
+Node.__rmul__ = lambda left, right: opset13.multiply(right, left)
+Node.__rdiv__ = lambda left, right: opset13.divide(right, left)
+Node.__rtruediv__ = lambda left, right: opset13.divide(right, left)
+Node.__eq__ = utils.deprecated(version="2025.3", message="Use ops.equal instead")(opset13.equal)
+Node.__ne__ = utils.deprecated(version="2025.3", message="Use ops.not_equal instead")(opset13.not_equal)
+Node.__lt__ = utils.deprecated(version="2025.3", message="Use ops.less instead")(opset13.less)
+Node.__le__ = utils.deprecated(version="2025.3", message="Use ops.less_equal instead")(opset13.less_equal)
+Node.__gt__ = utils.deprecated(version="2025.3", message="Use ops.greater instead")(opset13.greater)
+Node.__ge__ = utils.deprecated(version="2025.3", message="Use ops.greater_equal instead")(opset13.greater_equal)
+


we don't need it here. You can extend it in one place and re-use it overall.
Let say in py file for Node class that is more logical to have

I fully agree with your comment. I created a task to improve this.
This PR is simply about moving functionality from openvino/runtime/__init__ to openvino/__init__ so let's discuss it as a separate issue.

Let us do in this PR. It is normal process to get feedback and apply it in right away. Moreover, it is slight change and not a big deal:)

Signed-off-by: Alicja Miloszewska <[email protected]>

…into almilosz/alias-classes Signed-off-by: Alicja Miloszewska <[email protected]>

src/bindings/python/src/openvino/runtime/__init__.py

Signed-off-by: Alicja Miloszewska <[email protected]>

rkazants

Please remove duplicating code with defining Node class methods.
Yan can simple define it in Node py file (that is logical) and use it overall

almilosz · 2024-12-18T14:20:02Z

Node class is extended in a few places (and the code duplicates) because openvino, ovc and benchmark_tool inits have to be the same. There won't be a situation when somebody forget to update one of these files.
Unfortunately there are still some samples in jenkins that depend on mo init and openvino_dev init. That's why those two are also updated.

Please remove duplicating code with defining Node class methods. Yan can simple define it in Node py file (that is logical) and use it overall

I agree with you, but I don't want to introduce any new logic or new namespace in this PR. Let's discuss it separately. I created a task to not forget about it :)

@akuporos

p-wysocki

LGTM, one concerning thing is the specific opset13 imports in the main __init__.py file, but in my opinion it's out of scope of this PR.

p-wysocki · 2024-12-18T15:04:51Z

src/bindings/python/src/openvino/__init__.py

+Node.__add__ = opset13.add
+Node.__sub__ = opset13.subtract
+Node.__mul__ = opset13.multiply
+Node.__div__ = opset13.divide
+Node.__truediv__ = opset13.divide
+Node.__radd__ = lambda left, right: opset13.add(right, left)
+Node.__rsub__ = lambda left, right: opset13.subtract(right, left)
+Node.__rmul__ = lambda left, right: opset13.multiply(right, left)
+Node.__rdiv__ = lambda left, right: opset13.divide(right, left)
+Node.__rtruediv__ = lambda left, right: opset13.divide(right, left)


Why do we have opset13 here specifically? I'm not blocking the PR, it's a separate discussion.

There's no mechanism to get the "newest opset" op (which I think would be a good addition to the API), but still, there has to be room for improvement here. We may introduce a new version for one of these ops, for any reason, it's an edge case to remember that it also should be bumped here.

Moreover, these arithmetic methods are semantically incorrect to have. That is because we can't sum up nodes that have more one outputs.
It is more logical to define such methods for ov::Output (corresponding to tensors).

rkazants · 2024-12-18T15:24:24Z

I agree with you, but I don't want to introduce any new logic or new namespace in this PR. Let's discuss it separately. I created a task to not forget about it :)

I don't think this is a right approach to create task for review comments that can be applied easily by extra commit. Let us do this in this PR because the task can be forgotten, postponed for long time and closed finally as not implemented. Moreover, in that logic you have weak spot using opset13 that is repeating in several places. Let us have this weak spot just in one place that will be more easy to fix further.

rkazants

still not convinced

akuporos

I give the green light with one condition: let's rollback changes for Node magic methods and move them from __init__ in a separate PR.
This PR blocks #27694, but magic methods IMHO doesn't, so they can be moved after holidays.

Signed-off-by: Alicja Miloszewska <[email protected]>

…envinotoolkit#28085) ### Details: - Add imports from _pyopenivno on the top of the openvino `__init__.py` - Move `Node` binary operators support at the bottom of openvino `__init__.py` Classes and functions available in `openvino/runtime` will match those in flatten `openvino` ### Tickets: - [CVS-129453](https://jira.devtools.intel.com/browse/CVS-129453) --------- Signed-off-by: Alicja Miloszewska <[email protected]> Co-authored-by: Michal Lukaszewski <[email protected]>

…pace (openvinotoolkit#28085)" This reverts commit d0a8a39.

### Reverted: - #28062 (Guilty one) - #28085 (Relies on Guilty one) - #28166 (Relies on Guilty one)

### Details: Add deprecation warning for `openvino.runtime` that will be shown **ONCE** when sb will access runtime functionality for the first time. Examples: ![Screenshot 2025-01-10 114701](https://github.com/user-attachments/assets/4229ede3-3b86-418a-8dbf-f230ff91983b) ![warn_updated](https://github.com/user-attachments/assets/4b4a7c56-c5ca-4b7b-8b34-f89f3a4bc627) `openvino.runtime` funtionality was added to openvino namespace in these PRs: - #27785 - #27902 - #28007 - #28062 - #28085 Internal calls in `openvino` module also triggered warning, so they were updated: - #28166 - #28356 ### Tickets: - [CVS-129451](https://jira.devtools.intel.com/browse/CVS-129451) --------- Signed-off-by: Alicja Miloszewska <[email protected]> Co-authored-by: Anastasia Kuporosova <[email protected]>

### Reverted: - openvinotoolkit#28062 (Guilty one) - openvinotoolkit#28085 (Relies on Guilty one) - openvinotoolkit#28166 (Relies on Guilty one)

…7694) ### Details: Add deprecation warning for `openvino.runtime` that will be shown **ONCE** when sb will access runtime functionality for the first time. Examples: ![Screenshot 2025-01-10 114701](https://github.com/user-attachments/assets/4229ede3-3b86-418a-8dbf-f230ff91983b) ![warn_updated](https://github.com/user-attachments/assets/4b4a7c56-c5ca-4b7b-8b34-f89f3a4bc627) `openvino.runtime` funtionality was added to openvino namespace in these PRs: - openvinotoolkit#27785 - openvinotoolkit#27902 - openvinotoolkit#28007 - openvinotoolkit#28062 - openvinotoolkit#28085 Internal calls in `openvino` module also triggered warning, so they were updated: - openvinotoolkit#28166 - openvinotoolkit#28356 ### Tickets: - [CVS-129451](https://jira.devtools.intel.com/browse/CVS-129451) --------- Signed-off-by: Alicja Miloszewska <[email protected]> Co-authored-by: Anastasia Kuporosova <[email protected]>

almilosz added 3 commits December 16, 2024 20:41

Add classes to match runtime

1107ab4

Signed-off-by: Alicja Miloszewska <[email protected]>

Merge branch 'master' of https://github.com/openvinotoolkit/openvino …

1672ef6

…into almilosz/alias-classes Signed-off-by: Alicja Miloszewska <[email protected]>

Update benchmark_tool and ovc

ea87679

Signed-off-by: Alicja Miloszewska <[email protected]>

almilosz requested review from a team as code owners December 16, 2024 20:29

almilosz added the do_not_review label Dec 16, 2024

github-actions bot added category: Python API OpenVINO Python bindings category: tools OpenVINO C++ / Python tools category: OVC OVC tool labels Dec 16, 2024

almilosz added 3 commits December 17, 2024 09:48

Add Node

7ca37b5

Signed-off-by: Alicja Miloszewska <[email protected]>

Merge branch 'master' into almilosz/alias-classes

cbe6fa0

Merge branch 'master' of https://github.com/openvinotoolkit/openvino …

4ac92ea

…into almilosz/alias-classes Signed-off-by: Alicja Miloszewska <[email protected]>

almilosz added the do_not_merge label Dec 17, 2024

Fix for test in e2e-tests-linux-ubuntu22-e2e that uses MORunner

586fd4d

Signed-off-by: Alicja Miloszewska <[email protected]>

almilosz requested a review from a team as a code owner December 17, 2024 13:39

Align openvino_dev init with mo init

32b63b9

Signed-off-by: Alicja Miloszewska <[email protected]>

rkazants reviewed Dec 17, 2024

View reviewed changes

almilosz added 2 commits December 17, 2024 16:03

Align mo and ov dev inits with ov init

22d0a33

Signed-off-by: Alicja Miloszewska <[email protected]>

Merge branch 'master' of https://github.com/openvinotoolkit/openvino …

6514878

…into almilosz/alias-classes Signed-off-by: Alicja Miloszewska <[email protected]>

almilosz removed do_not_review do_not_merge labels Dec 18, 2024

almilosz requested review from akuporos and p-wysocki December 18, 2024 07:32

akuporos reviewed Dec 18, 2024

View reviewed changes

src/bindings/python/src/openvino/runtime/__init__.py Outdated Show resolved Hide resolved

Move deprecated Node binary operations back to runtime init

8971501

Signed-off-by: Alicja Miloszewska <[email protected]>

almilosz requested review from akuporos and rkazants December 18, 2024 13:12

rkazants requested changes Dec 18, 2024

View reviewed changes

almilosz requested a review from rkazants December 18, 2024 14:20

p-wysocki approved these changes Dec 18, 2024

View reviewed changes

rkazants requested changes Dec 18, 2024

View reviewed changes

akuporos approved these changes Dec 18, 2024

View reviewed changes

Rollback changes to Node binary ops support

4e93d95

Signed-off-by: Alicja Miloszewska <[email protected]>

akuporos requested a review from rkazants December 18, 2024 16:16

rkazants approved these changes Dec 18, 2024

View reviewed changes

Merge branch 'master' into almilosz/alias-classes

6b31b3f

mlukasze enabled auto-merge December 19, 2024 05:51

Merge branch 'master' into almilosz/alias-classes

0534c48

mlukasze added this pull request to the merge queue Dec 19, 2024

Merged via the queue into openvinotoolkit:master with commit d0a8a39 Dec 19, 2024
203 of 204 checks passed

ilya-lavrenov added a commit to ilya-lavrenov/openvino that referenced this pull request Dec 24, 2024

Revert "[Py OV] Alias classes and functions in flatten openvino names…

ffc9b28

…pace (openvinotoolkit#28085)" This reverts commit d0a8a39.

ilya-lavrenov mentioned this pull request Dec 24, 2024

Reverted python API non BW compatible changes #28190

Merged

github-merge-queue bot pushed a commit that referenced this pull request Dec 24, 2024

Reverted python API non BW compatible changes (#28190)

68b1f54

### Reverted: - #28062 (Guilty one) - #28085 (Relies on Guilty one) - #28166 (Relies on Guilty one)

almilosz mentioned this pull request Jan 9, 2025

[Py OV] Add deprecation warning for runtime module #27694

Merged

almilosz deleted the almilosz/alias-classes branch January 30, 2025 09:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Py OV] Alias classes and functions in flatten openvino namespace #28085

[Py OV] Alias classes and functions in flatten openvino namespace #28085

almilosz commented Dec 16, 2024 •

edited

Loading

rkazants Dec 17, 2024

almilosz Dec 18, 2024

rkazants Dec 18, 2024 •

edited

Loading

rkazants left a comment

almilosz commented Dec 18, 2024

p-wysocki left a comment

p-wysocki Dec 18, 2024

rkazants Dec 18, 2024

rkazants commented Dec 18, 2024 •

edited

Loading

rkazants left a comment

akuporos left a comment •

edited

Loading

[Py OV] Alias classes and functions in flatten openvino namespace #28085

[Py OV] Alias classes and functions in flatten openvino namespace #28085

Conversation

almilosz commented Dec 16, 2024 • edited Loading

Details:

Tickets:

rkazants Dec 17, 2024

Choose a reason for hiding this comment

almilosz Dec 18, 2024

Choose a reason for hiding this comment

rkazants Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

rkazants left a comment

Choose a reason for hiding this comment

almilosz commented Dec 18, 2024

p-wysocki left a comment

Choose a reason for hiding this comment

p-wysocki Dec 18, 2024

Choose a reason for hiding this comment

rkazants Dec 18, 2024

Choose a reason for hiding this comment

rkazants commented Dec 18, 2024 • edited Loading

rkazants left a comment

Choose a reason for hiding this comment

akuporos left a comment • edited Loading

Choose a reason for hiding this comment

almilosz commented Dec 16, 2024 •

edited

Loading

rkazants Dec 18, 2024 •

edited

Loading

rkazants commented Dec 18, 2024 •

edited

Loading

akuporos left a comment •

edited

Loading