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.

Sign up for GitHub

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

Jump to bottom

[ONNX] Add ReduceLogSum-11,13,18 #23508

Merged
merged 11 commits into from
Mar 28, 2024
Merged

[ONNX] Add ReduceLogSum-11,13,18 #23508

merged 11 commits into from
Mar 28, 2024

Conversation

mory91
Copy link
Contributor

@mory91 mory91 commented Mar 18, 2024 •
edited
Loading

@mory91 mory91 requested a review from a team as a code owner March 18, 2024 01:42
@github-actions github-actions bot added the category: ONNX FE OpenVINO ONNX FrontEnd label Mar 18, 2024
@mory91 mory91 changed the title Add ReduceLogSum-11,13,18 Add ReduceLogSum-13,18 Mar 18, 2024
@mory91 mory91 changed the title Add ReduceLogSum-13,18 Add ReduceLogSum-11,13,18 Mar 18, 2024
@mory91 mory91 force-pushed the onnx_reducelogsum branch 3 times, most recently from accc95a to 76282fc Compare March 18, 2024 08:21
@ilya-lavrenov ilya-lavrenov added the ExternalPR External contributor label Mar 18, 2024
@mory91 mory91 force-pushed the onnx_reducelogsum branch 2 times, most recently from d2dd405 to f4dc5bc Compare March 18, 2024 17:17
@mory91 mory91 changed the title Add ReduceLogSum-11,13,18 [onnx] Add ReduceLogSum-11,13,18 Mar 18, 2024
@mory91 mory91 changed the title [onnx] Add ReduceLogSum-11,13,18 [ONNX] Add ReduceLogSum-11,13,18 Mar 18, 2024
@mory91 mory91 force-pushed the onnx_reducelogsum branch 2 times, most recently from 8d60120 to 9d17b57 Compare March 18, 2024 22:37
@mory91 mory91 force-pushed the onnx_reducelogsum branch 4 times, most recently from 0ee4ec9 to 0b00871 Compare March 19, 2024 17:21
@mitruska mitruska self-assigned this Mar 19, 2024
mitruska
mitruska reviewed Mar 19, 2024
View reviewed changes
Copy link
Contributor

@mitruska mitruska left a comment

Choose a reason for hiding this comment

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

Hello @mory91, thank you for this contribution!
The import code looks logically correct, I've suggested some improvements to avoid code redundancy.

src/frontends/onnx/frontend/src/op/reduce.cpp Outdated Show resolved Hide resolved
src/frontends/onnx/frontend/src/ops_bridge.cpp Outdated Show resolved Hide resolved
src/frontends/onnx/frontend/src/op/reduce.hpp Outdated Show resolved Hide resolved
src/frontends/onnx/tests/models/reduce_log_sum_18.prototxt
Comment on lines +1 to +8
ir_version: 3
producer_name: "OpenVINO ONNX Frontend"
graph {
node {
input: "A"
output: "B"
op_type: "ReduceLogSum"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be great to add some tests related to the features added in the ReduceLogSum-18 (noop_with_empty_axes attr and axes as input).
https://onnx.ai/onnx/operators/text_diff_ReduceLogSum_13_18.html#d2h-421101

src/frontends/onnx/frontend/src/op/reduce.cpp Outdated Show resolved Hide resolved
src/frontends/onnx/tests/onnx_import.in.cpp Show resolved Hide resolved
@mitruska mitruska removed their assignment Mar 19, 2024
@mory91 mory91 force-pushed the onnx_reducelogsum branch 4 times, most recently from ad5573e to 42812bf Compare March 19, 2024 21:49
@mory91 mory91 requested a review from mitruska March 20, 2024 07:12
@mlukasze
Copy link
Contributor

hey @mory91 please correct formatting to pass clang tests

@mlukasze mlukasze requested review from mitruska and removed request for a team and zKulesza March 26, 2024 06:11
@gkrivor gkrivor mentioned this pull request Mar 26, 2024
Closed
@mitruska
Copy link
Contributor

Current CI issues are not related to the PR changes, it should be fixed soon, no action required so far.

@mitruska
Copy link
Contributor

build_jenkins

@mlukasze mlukasze enabled auto-merge March 28, 2024 05:35
@mlukasze mlukasze added this pull request to the merge queue Mar 28, 2024
Merged via the queue into openvinotoolkit:master with commit e4b82a5 Mar 28, 2024
108 checks passed
@mlukasze mlukasze added this to the 2024.1 milestone Apr 5, 2024
bbielawx pushed a commit to bbielawx/openvino that referenced this pull request Apr 12, 2024
@mory91 @mitruska
@gkrivor @bbielawx
[ONNX] Add ReduceLogSum-11,13,18 (openvinotoolkit#23508)
791950b 
### Details:
 - Extended ReduceLogSum by opsets 11,13,18


### Tickets:
 - Closes openvinotoolkit#20561

---------

Co-authored-by: Katarzyna Mitrus <katarzyna.mitrus@intel.com>
Co-authored-by: Georgy Krivoruchko <georgy.krivoruchko@intel.com>
alvoron pushed a commit to alvoron/openvino that referenced this pull request Apr 29, 2024
@mory91 @mitruska
@gkrivor @alvoron
[ONNX] Add ReduceLogSum-11,13,18 (openvinotoolkit#23508)
918d938 
### Details:
 - Extended ReduceLogSum by opsets 11,13,18


### Tickets:
 - Closes openvinotoolkit#20561

---------

Co-authored-by: Katarzyna Mitrus <katarzyna.mitrus@intel.com>
Co-authored-by: Georgy Krivoruchko <georgy.krivoruchko@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gkrivor gkrivor gkrivor approved these changes

@mitruska mitruska mitruska approved these changes

@mmikolajcz mmikolajcz Awaiting requested review from mmikolajcz mmikolajcz was automatically assigned from openvinotoolkit/openvino-pytorch-frontend-maintainers

@PiotrKrzem PiotrKrzem Awaiting requested review from PiotrKrzem PiotrKrzem was automatically assigned from openvinotoolkit/openvino-pytorch-frontend-maintainers

Assignees
No one assigned
Labels
category: ONNX FE OpenVINO ONNX FrontEnd ExternalPR External contributor
Projects
None yet
Milestone
2024.1
Development

Successfully merging this pull request may close these issues.

[Good First Issue]: Align behavior of ONNX Frontend function ReduceLogSum-11, 13, 18 with original framework
5 participants
@mory91 @mlukasze @mitruska @gkrivor @ilya-lavrenov