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

[Docs][PyOV] Documentation for Custom Python Operation #25615

Open
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

akuporos
Copy link
Contributor

Details:

  • Extend documentation with python examples

Tickets:

@akuporos akuporos requested a review from a team as a code owner July 18, 2024 05:41
@akuporos akuporos requested review from akopytko and removed request for a team July 18, 2024 05:41
@github-actions github-actions bot added the category: docs OpenVINO documentation label Jul 18, 2024
@akuporos akuporos added this to the 2024.4 milestone Jul 18, 2024
@akuporos akuporos force-pushed the akup/custom-op-docs branch from 073084e to e0f40f9 Compare July 18, 2024 06:55
@mlukasze mlukasze added this pull request to the merge queue Jul 26, 2024
@mlukasze mlukasze removed this pull request from the merge queue due to a manual request Jul 26, 2024
Copy link
Contributor

This PR will be closed in a week because of 2 weeks of no activity.

@github-actions github-actions bot added the Stale label Nov 12, 2024
@akuporos akuporos added no_stale Do not mark as stale and removed Stale labels Nov 13, 2024
@kblaszczak-intel
Copy link
Contributor

Hey everyone, can we finalize this one? It has been hanging for a while and Anastasia did a lot of good work here, would be a shame to let it go to waste. Even if it is not perfect, maybe we could go through with some polishing, merge, publish, and then continue improving in a separate PR?
Let me know if I can help.

@kblaszczak-intel
Copy link
Contributor

@akuporos
@slyalin
Just a nudge ;) Can I help with this one in any way?

@akuporos
Copy link
Contributor Author

Hi @kblaszczak-intel ,

I'd like to complete this PR first, and then return back to this one.
It'd help me to fully complete this PR

@mlukasze mlukasze added the WIP work in progress label Nov 29, 2024
@github-actions github-actions bot added the category: docs_snippets OpenVINO docs snippets (docs/snippets) label Jan 16, 2025
5. Override the ``visit_attributes`` method, which enables serialization and deserialization of operation attributes. An ``AttributeVisitor`` is passed to the method, and the implementation is expected to walk over all the attributes in the op using the type-aware ``on_attribute`` helper. Helpers are already implemented for standard C++ types like ``int64_t``, ``float``, ``bool``, ``vector``, and for existing OpenVINO defined types.
There are some methods that can also be overridden if needed:

4. Override the ``clone_with_new_inputs`` method, which enables graph manipulation routines to create copies of this operation and connect it to different nodes during optimization.
Copy link
Contributor

Choose a reason for hiding this comment

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

The description doesn't provide any reason why this function is optional. If I don't implement it, what does happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the default ones will be called.
but overall I removed this phrase

Copy link
Contributor

Choose a reason for hiding this comment

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

So, now clone_with_new_inputs is mandatory according to the documentation? That was one of the things we were trying to fix for Python (this is one of the "Python Exclusives" in this case). And now we are not emphasizing it.

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've edited the sentence and added remark that it is optional

When Python API is used, there is no way to implement a custom OpenVINO operation. Even if custom OpenVINO operation is implemented in C++ and loaded into the runtime by a shared library, there is still no way to add a frontend mapping extension that refers to this custom operation. In this case, use C++ shared library approach to implement both operations semantics and framework mapping.

Python can still be used to map and decompose operations when only operations from the standard OpenVINO operation set are used.
If custom OpenVINO operation is implemented in C++ and loaded into the runtime using Python API by a shared library, there is no way to add a frontend mapping extension that refers to this custom operation. In this case, use C++ shared library approach to implement both operations semantics and framework mapping.
Copy link
Contributor

Choose a reason for hiding this comment

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

This limitation exists for both C++ and Python.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@akuporos akuporos force-pushed the akup/custom-op-docs branch from 45d9450 to 7ecd86d Compare January 30, 2025 00:12
@akuporos akuporos requested a review from slyalin January 30, 2025 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: docs_snippets OpenVINO docs snippets (docs/snippets) category: docs OpenVINO documentation no_stale Do not mark as stale WIP work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants