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

[ONNX] Reduce* refactoring #23429

Merged
merged 7 commits into from
Mar 14, 2024
Merged

Conversation

gkrivor
Copy link
Contributor

@gkrivor gkrivor commented Mar 13, 2024

Details:

  • Refactored source code to be able co-work with others
  • Removed unnecessary comments
  • Added check for a wrong input type
  • Added tests for an exception for wrong input type

Tickets:

  • 125493

Comment on lines +93 to +96
const std::set<element::Type> supported_types_v1 =
{element::u32, element::u64, element::i32, element::i64, element::f16, element::f32, element::f64};
const std::set<element::Type> supported_types_v2 =
{element::u32, element::u64, element::i32, element::i64, element::f16, element::f32, element::f64, element::bf16};
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we do need to check input types to Reduce operation. That is because we assume input model is originally consistent (it comes with correct input type) and our Reduce operation should work for all of this operations

Copy link
Contributor

Choose a reason for hiding this comment

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

This way we can explore limitation in core part and request to extend on new types if needed.
Let us say if some type not supported by core (or plugin, etc.) is needed by some model, then we should insert Convert operation that is not good.
We should rather to avoid any limitations in our part, assume that input model is consistent, and encourage other components avoid limitations.

@gkrivor gkrivor added this pull request to the merge queue Mar 14, 2024
Merged via the queue into openvinotoolkit:master with commit 149381e Mar 14, 2024
105 checks passed
@gkrivor gkrivor deleted the onnx_reduce_ref branch March 14, 2024 12:07
alvoron pushed a commit to alvoron/openvino that referenced this pull request Apr 29, 2024
### Details:
 - Refactored source code to be able co-work with others
 - Removed unnecessary comments
 - Added check for a wrong input type
 - Added tests for an exception for wrong input type

### Tickets:
 - 125493
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants