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

Extend ONNX Frontend with Shape-15 operator #20194

Closed
mitruska opened this issue Oct 2, 2023 · 31 comments · Fixed by #23492
Closed

Extend ONNX Frontend with Shape-15 operator #20194

mitruska opened this issue Oct 2, 2023 · 31 comments · Fixed by #23492
Assignees
Labels
category: ONNX FE OpenVINO ONNX FrontEnd good first issue Good for newcomers no_stale Do not mark as stale
Milestone

Comments

@mitruska
Copy link
Contributor

mitruska commented Oct 2, 2023

Context

Neural networks are graphs consisting of nodes called operators. Each operator corresponds to a mathematical function, usually described in framework's documentation or an AI standard, such as ONNX.

OpenVINO ONNX Frontend is a component responsible for working with ONNX graphs and requires implementation of different ONNX operators in order to use ONNX models.

This task requires extending OpenVINO ONNX Frontend with Shape-15 (ONNX Shape operator since ONNX Opset 15).
ONNX Shape operator has been extended with additional "start, end" attributes, what should be reflected in the conversion logic.
Currently those attributes are ignored by ONNX FE, and ONNX Shape is converted directly to ov::ShapeOf,
but it can be supported at the ONNX FE conversion step, with additional ov::Slice layer.

Necessary help will be provided by ONNX Fronted team.

Operator specification

Operator details can be found in ONNX Operators.

Todo list

  1. Update .hpp and .cpp files:
    /openvino/src/frontends/onnx/frontend/src/op/shape.hpp
    /openvino/src/frontends/onnx/frontend/src/op/shape.cpp
  2. Prepare an implementation of this operator in form of a function. It should be placed in set_15 namespace.
  3. Register the next version of the Shape operator: REGISTER_OPERATOR("Shape", 15, shape); in ops_bridge.cpp while keeping alphabetical order
  4. Create ONNX Shape-15 test model(s) in ONNX models directory. OpenVINO test infrastructure then converts prototxt files to ONNX models - you will use those models later in tests. Double check the ONNX opset version of the model that should be at least 15.
  5. Add tests covering all use cases here

More details in adding operators to ONNX Frontend guide

Resources

Example PRs

Tips

Proper implementation should enable the following tests:

xfail_issue_63138 = xfail_test(reason="Missing ONNX Shape-15 support")

xfail_issue_63138,
"OnnxBackendNodeModelTest.test_shape_end_1_cpu",
"OnnxBackendNodeModelTest.test_shape_end_negative_1_cpu",
"OnnxBackendNodeModelTest.test_shape_start_1_cpu",
"OnnxBackendNodeModelTest.test_shape_start_1_end_2_cpu",
"OnnxBackendNodeModelTest.test_shape_start_1_end_negative_1_cpu",
"OnnxBackendNodeModelTest.test_shape_start_negative_1_cpu",
),

Also for compatibility API:

Contact points

@mitruska
@p-wysocki
@gkrivor
Don't hesitate to reach out, we're here to help!

@mitruska mitruska added good first issue Good for newcomers no_stale Do not mark as stale labels Oct 2, 2023
@p-wysocki p-wysocki moved this to Contrubutors needed in Good first issues Oct 3, 2023
@hotcheetofiend
Copy link

hotcheetofiend commented Oct 5, 2023

@mitruska If no one has been assigned yet, could I work on this issue?

@p-wysocki p-wysocki moved this from Contrubutors needed to Assigned in Good first issues Oct 5, 2023
@p-wysocki
Copy link
Contributor

Hello @hotcheetofiend, thanks for taking a look! I assigned you.

@p-wysocki
Copy link
Contributor

Hi @hotcheetofiend, are you still working on it? I'm updating the tasks' statuses.

@hotcheetofiend
Copy link

Hi @hotcheetofiend, are you still working on it? I'm updating the tasks' statuses.

Yes; sorry! I will try to get it done by Friday.

@mitruska
Copy link
Contributor Author

Hello @hotcheetofiend, let us know if you find this task challenging.
We are here to help 💡

@mitruska mitruska added the category: ONNX FE OpenVINO ONNX FrontEnd label Oct 31, 2023
@p-wysocki
Copy link
Contributor

Hi @hotcheetofiend, are you still working on this or can we reopen the issue for pickup by other contributors?

@p-wysocki p-wysocki moved this from Assigned to Contributors Needed in Good first issues Nov 24, 2023
@BharathSatheeshKumar
Copy link

.take

Copy link
Contributor

github-actions bot commented Dec 8, 2023

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

@BharathSatheeshKumar
Copy link

May I get guided by anyone. Like i just want a simple description on the issue so that i can work on it.
Even tho im new i can give results

@mitruska
Copy link
Contributor Author

mitruska commented Dec 8, 2023

Hello @BharathSatheeshKumar, thank you for taking this issue, the idea is explained in the issue description above:

This task requires extending OpenVINO ONNX Frontend with Shape-15 (ONNX Shape operator since ONNX Opset 15).
ONNX Shape operator has been extended with additional "start, end" attributes, what should be reflected in the conversion logic.
Currently those attributes are ignored by ONNX FE, and ONNX Shape is converted directly to ov::ShapeOf,
but it can be supported at the ONNX FE conversion step, with additional ov::Slice layer.

In other words, the goal is to add support for the start and end attributes of the ONNX Shape-15, by conversion to Slice(ShapeOf(data), start, end, 1, 0) subgraph.
Also please take a look at the TODO steps and example PRs, where related files are mentioned.

@BharathSatheeshKumar
Copy link

Thank You.
And Do I Get a deadline for this so that i can give my best at anycost

@mitruska
Copy link
Contributor Author

mitruska commented Dec 8, 2023

Happy to help 🚀
Currently there is no specific deadline for this task, but you can share your approximate time estimation and let us know about any troubles.

@BharathSatheeshKumar
Copy link

I've updated the hpp and cpp files but cant create a PR. Help me out
I'll let you know about the deadline within a few days of working with this issue

@mitruska
Copy link
Contributor Author

mitruska commented Dec 8, 2023

Step by step contribution guide, including how to create a Pull Request [PR], can be found in the Resources section of the issue description, please follow:

blog post on contributing to OpenVINO

@mitruska mitruska moved this from Contributors Needed to Assigned in Good first issues Dec 11, 2023
@p-wysocki
Copy link
Contributor

Hello @BharathSatheeshKumar, do you need any help?

Just yesterday our CONTRIBUTING.md has been updated with a technical guide - I highly recommend checking it out. :)

@BharathSatheeshKumar
Copy link

Yea sure will check that out and will let you guys know soon.
And i still have problems in creating a pr for this base even tho I followed what @mitruska sent

@BharathSatheeshKumar
Copy link

PR link?

@p-wysocki
Copy link
Contributor

Hello @BharathSatheeshKumar, could you please let us know what issues you're having?

@BharathSatheeshKumar
Copy link

Just now sorted the problem thanks....and will update within a day or so

@BharathSatheeshKumar
Copy link

i cant access the main file any ideas?

@p-wysocki
Copy link
Contributor

Hi @BharathSatheeshKumar, I think we may need more details. Could you please join Intel DevHub Discord and ask your questions there? It will be easier for both sides.

@p-wysocki
Copy link
Contributor

Due to long inactivity I'm unassigning the issue for other contributors. If you're still working on this task @BharathSatheeshKumar please let us know, you'll be reassigned.

@p-wysocki p-wysocki moved this from Assigned to Contributors Needed in Good first issues Feb 5, 2024
@AlexFierro9
Copy link

.take

Copy link
Contributor

github-actions bot commented Feb 7, 2024

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

@mlukasze mlukasze moved this from Contributors Needed to Assigned in Good first issues Feb 7, 2024
@BharathSatheeshKumar
Copy link

sorry for the delay tho
let me work with @AlexFierro9 for the next project

@p-wysocki
Copy link
Contributor

Hello @AlexFierro9, are you still working on that issue?

@p-wysocki p-wysocki moved this from Assigned to Contributors Needed in Good first issues Mar 12, 2024
@MeeCreeps
Copy link
Contributor

.take

Copy link
Contributor

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

@p-wysocki p-wysocki moved this from Contributors Needed to Assigned in Good first issues Mar 12, 2024
@MeeCreeps
Copy link
Contributor

Hi @p-wysocki , I have raised PR #23492 for this issue, please let me know if any modifications are needed.

@mlukasze mlukasze linked a pull request Mar 18, 2024 that will close this issue
@mlukasze
Copy link
Contributor

thanks @MeeCreeps!
@gkrivor will take a look soon

@mlukasze mlukasze moved this from Assigned to In Review in Good first issues Mar 18, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 29, 2024
@github-project-automation github-project-automation bot moved this from In Review to Closed in Good first issues Mar 29, 2024
@mlukasze mlukasze added this to the 2024.1 milestone Mar 29, 2024
@mitruska
Copy link
Contributor Author

Hello @MeeCreeps your PR has been reviewed, approved and merged 🚀
Thanks, Looking forward to your further contribution!

bbielawx pushed a commit to bbielawx/openvino that referenced this issue Apr 12, 2024
alvoron pushed a commit to alvoron/openvino that referenced this issue Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: ONNX FE OpenVINO ONNX FrontEnd good first issue Good for newcomers no_stale Do not mark as stale
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants