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

Add SegmentMax-16 to Core #28698

Merged
merged 26 commits into from
Feb 13, 2025
Merged

Add SegmentMax-16 to Core #28698

merged 26 commits into from
Feb 13, 2025

Conversation

p-wysocki
Copy link
Contributor

@p-wysocki p-wysocki commented Jan 27, 2025 •
edited
Loading

@p-wysocki
WIP
d338b7e 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki
WIP
462c43f 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki
Cleanup
e3e44e2 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki
Merge branch 'master' of https://github.com/openvinotoolkit/openvino …
adcce7d 
…into segmentmax_core

Signed-off-by: p-wysocki <[email protected]>
@p-wysocki p-wysocki added this to the 2025.1 milestone Jan 27, 2025
@p-wysocki p-wysocki requested review from a team as code owners January 27, 2025 15:33
@github-actions github-actions bot added category: Core OpenVINO Core (aka ngraph) category: CPU OpenVINO CPU plugin category: CPP API OpenVINO CPP API bindings labels Jan 27, 2025
@p-wysocki
Fix two tests
1affe5d 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki p-wysocki mentioned this pull request Jan 28, 2025
Merged
@p-wysocki
CR WIP
094131d 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki
WIP CR
f8c9919 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki
WIP CR
f841c4c 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki
WIP
3e882ed 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki
Clenaup
7cef3b5 
Signed-off-by: p-wysocki <[email protected]>
mitruska
mitruska requested changes Jan 31, 2025
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.

The shape inference logic of the case when "num_segments" input is provided as Parameter needs adjustments (details in the comments).

@p-wysocki
Merge branch 'master' of https://github.com/openvinotoolkit/openvino …
fb76824 
…into segmentmax_core

Signed-off-by: p-wysocki <[email protected]>
@p-wysocki
Apply CR
b8645d3 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki p-wysocki requested a review from mitruska February 3, 2025 14:40
@p-wysocki p-wysocki mentioned this pull request Feb 3, 2025
Merged
p-wysocki and others added 3 commits February 6, 2025 00:18
@p-wysocki @mitruska
Apply suggestions from code review
61979d0 
Co-authored-by: Katarzyna Mitrus <[email protected]>
@p-wysocki
Apply CR
ae37028 
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki
Merge branch 'segmentmax_core' of https://github.com/p-wysocki/openvino…
303f383 
… into segmentmax_core

Signed-off-by: p-wysocki <[email protected]>
@p-wysocki p-wysocki requested a review from praasz February 5, 2025 23:53
praasz
praasz approved these changes Feb 7, 2025
View reviewed changes
src/core/shape_inference/include/segment_max_shape_inference.hpp
std::vector<TRShape> shape_infer(const SegmentMax* op,
const std::vector<TShape>& input_shapes,
const ITensorAccessor& tensor_accessor = make_tensor_accessor()) {
NODE_VALIDATION_CHECK(op, input_shapes.size() == 2 || input_shapes.size() == 3);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
NODE_VALIDATION_CHECK(op, input_shapes.size() == 2 || input_shapes.size() == 3);
OPENVINO_ASSERT(input_shapes.size() == 2 || input_shapes.size() == 3);

Consider apply if there will be any other change in this PR

@p-wysocki p-wysocki added this pull request to the merge queue Feb 12, 2025
github-merge-queue bot pushed a commit that referenced this pull request Feb 12, 2025
@p-wysocki @mitruska @rkazants
Add specification for SegmentMax-16 (#28103)
494d645 
### Details:
- Specification for `tf.math.segment_max`
(https://www.tensorflow.org/api_docs/python/tf/math/segment_max)

### Related PRs:
 - #28698
 - #28788

### Tickets:
 - CVS-158914

---------

Signed-off-by: p-wysocki <[email protected]>
Co-authored-by: Katarzyna Mitrus <[email protected]>
Co-authored-by: Roman Kazantsev <[email protected]>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 12, 2025
@p-wysocki p-wysocki added this pull request to the merge queue Feb 12, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 12, 2025
@p-wysocki p-wysocki enabled auto-merge February 12, 2025 14:39
@p-wysocki p-wysocki added this pull request to the merge queue Feb 12, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 12, 2025
@mlukasze mlukasze added this pull request to the merge queue Feb 13, 2025
Merged via the queue into openvinotoolkit:master with commit d455dd2 Feb 13, 2025
185 checks passed
This was referenced Feb 13, 2025
Open
Open
github-merge-queue bot pushed a commit that referenced this pull request Feb 17, 2025
@p-wysocki @rkazants
@praasz @mitruska
Add SegmentMax-16 reference implementation (#28788)
5de222d 
### Details:
 - Add reference implementation
 - Add tests

### Related PRs:
 - #28103
 - #28698
 - #28979
 - #28999

### Tickets:
 - CVS-158917

---------

Signed-off-by: p-wysocki <[email protected]>
Co-authored-by: Roman Kazantsev <[email protected]>
Co-authored-by: Pawel Raasz <[email protected]>
Co-authored-by: Katarzyna Mitrus <[email protected]>
@p-wysocki p-wysocki mentioned this pull request Feb 18, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Feb 18, 2025
@p-wysocki @rkazants
@praasz @mitruska
Add SegmentMax-16 reference implementation (#29047)
18e97b6 
### Details:
- The original PR
(#28788) has been
mistakenly force-merged due to a mistake in merge queue settings. It was
later reverted, so this is the "new" Ref PR.
 - Add reference implementation
 - Add tests

### Related PRs:
 - #28103
 - #28698
 - #28979
 - #28999

### Tickets:
 - CVS-158917

---------

Signed-off-by: p-wysocki <[email protected]>
Co-authored-by: Roman Kazantsev <[email protected]>
Co-authored-by: Pawel Raasz <[email protected]>
Co-authored-by: Katarzyna Mitrus <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mitruska mitruska mitruska approved these changes

@praasz praasz praasz approved these changes

@mmikolajcz mmikolajcz Awaiting requested review from mmikolajcz

@PiotrKrzem PiotrKrzem Awaiting requested review from PiotrKrzem

Assignees
No one assigned
Labels
category: Core OpenVINO Core (aka ngraph) category: CPP API OpenVINO CPP API bindings category: CPU OpenVINO CPU plugin
Projects
None yet
Milestone
2025.1
Development

Successfully merging this pull request may close these issues.
4 participants
@p-wysocki @mitruska @praasz @mlukasze