openvino icon indicating copy to clipboard operation
openvino copied to clipboard

[Good First Issue]: Align behavior of ONNX Frontend function ReduceLogSum-11, 13, 18 with original framework

Open gkrivor opened this issue 1 year ago • 5 comments

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 alignment between OpenVINO ONNX Frontend and original framework implementations of ReduceLogSum for next list of opsets: opset 11, opset 13, opset 18 Necessary help will be provided by ONNX Fronted team.

What needs to be done?

Operator details can be found in ONNX Operators More details can be found in ONNX Changelog: opset 11, opset 13, opset 18

  1. Function already has a common implementation in OpenVINO. First of all, you need to review a documentation and prepare a table with differences between versions. It could be, for instance, a missing property, extended/reduced coverage of existing property, etc...
  2. Copy existing implementation here to make it aligned with original framework (extend or reduce coverage of a common implementation). Copy of modified implementation should be in a defined opset, or in opset 1 in case it implements oldest implementation. Example of multi-opset operation.
  3. Register the function in ops_bridge.cpp while keeping alphabetical order
  4. Create 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
  5. Add tests covering all use cases here
  6. Check Python xfailed tests to find a test marked as a xfailed for added functionality. If any exist - remove corresponding lines and try to verify by using cmdline "python -m pytest -k name_of_test". More details in adding operators to ONNX Frontend guide

Example Pull Requests

No response

Resources

Contact points

@gkrivor

Ticket

No response

gkrivor avatar Oct 18 '23 12:10 gkrivor

Hi! I'm a first time contributor. Could this issue be assigned to me?

Hunter-bitbit avatar Feb 09 '24 07:02 Hunter-bitbit

It's yours now :)

mlukasze avatar Feb 09 '24 07:02 mlukasze

Hello @Hunter-bitbit, do you need any help from us? :)

p-wysocki avatar Feb 19 '24 13:02 p-wysocki

Hi @p-wysocki, some help would be great, yes. I'm mostly just having issues with understanding what needs to be done. I made the table of differences between versions, but steps 2 and 3 don't make much sense to me. I found the existing implementation but I don't know where to copy it to? For the registration I found a ReducedLogSum in there... do I have to specify the namespace it's in? I'm just very confused as to what I actually have to do since it seems like everything is already implemented... sorry for the hassle :\

Hunter-bitbit avatar Feb 21 '24 08:02 Hunter-bitbit

Hello @Hunter-bitbit, it may be the case that the operator is currently correctly aligned. Could you please give a brief summary of the changes between op versions and what OpenVINO has? That would simplify the discussion a lot. cc @gkrivor

p-wysocki avatar Feb 23 '24 13:02 p-wysocki

.take

mengbingrock avatar Mar 07 '24 22:03 mengbingrock

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

github-actions[bot] avatar Mar 07 '24 22:03 github-actions[bot]

Hi, The PR is created: 23522

mengbingrock avatar Mar 18 '24 19:03 mengbingrock