Skip to content

[ONNX] Introduce GraphIterator interface in frontend #32325

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
Merged
merged 11 commits into from
Oct 12, 2025
Merged

[ONNX] Introduce GraphIterator interface in frontend #32325

merged 11 commits into from
Oct 12, 2025

Conversation

mvafin
Copy link
Contributor

@mvafin mvafin commented Oct 7, 2025

Details:

  • Introduce GraphIterator for ONNX

Tickets:

@mvafin mvafin requested a review from a team as a code owner October 7, 2025 19:43
@github-actions github-actions bot added category: build OpenVINO cmake script / infra category: ONNX FE OpenVINO ONNX FrontEnd category: CPP API OpenVINO CPP API bindings labels Oct 7, 2025
@mvafin mvafin force-pushed the mvafin/onnx/delegate branch 2 times, most recently from 2f2d379 to 3addff7 Compare October 8, 2025 11:12
rkazants
rkazants reviewed Oct 9, 2025
View reviewed changes
mvafin added 2 commits October 9, 2025 13:45
@mvafin
Improve verbosity
c05b884 
Signed-off-by: Maxim Vafin <[email protected]>
@mvafin
Fix code style
ee61095 
Signed-off-by: Maxim Vafin <[email protected]>
@mvafin mvafin force-pushed the mvafin/onnx/delegate branch from 472760f to fca9980 Compare October 9, 2025 13:17
@mvafin
Remove tf related code
fca9980 
Signed-off-by: Maxim Vafin <[email protected]>
mvafin
mvafin commented Oct 9, 2025
View reviewed changes
@mvafin mvafin requested a review from bumbosiepsak October 10, 2025 08:47
bumbosiepsak
bumbosiepsak approved these changes Oct 10, 2025
View reviewed changes
Copy link
Contributor

@bumbosiepsak bumbosiepsak left a comment

Choose a reason for hiding this comment

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

I managed to review a (smaller) half of the feature. I am approving for the sake of having real-world tests after merging. Let's discuss how to proceed/polish it further.

src/frontends/onnx/tests/graph_iterator.cpp Outdated

#include <openvino/openvino.hpp>

#include "../frontend/src/core/graph_iterator_proto.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, handle it on the buildsystem/CMake level, so that reaching paths from different trees is not needed:

// Proper way:
// - let the "openvino::frontend::onnx" target export the "INTERFACE_INCLUDE_DIRECTORIES" property
// - linking the test executable with that target will import all necessary include paths
#include "frontend/src/core/graph_iterator_proto.hpp"

From a quick inspection, I think it's already happening. Maybe worth trying (and removing the ../ part from the path)?

src/frontends/onnx/tests/CMakeLists.txt
onnx_reader_external_data.cpp
skip_tests_config.cpp)
skip_tests_config.cpp
../frontend/src/core/graph_iterator_proto.cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this should not be built here. Consider creating a proper helper target in frontend/src/core that creates an object libarary (or a static one) from graph_iterator_proto.cpp and decoder_proto.cpp. Then link with it here.

src/frontends/onnx/frontend/src/input_model.hpp
const std::shared_ptr<TelemetryExtension>& telemetry = {});
explicit InputModel(const ov::frontend::onnx::GraphIterator::Ptr& graph_iterator, InputModel* parent_model);

///// Searching for places /////
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: make it proper Doxygen:

/// Searching for inputs
std::vector<ov::frontend::Place::Ptr> get_inputs() const override;

/// Searching for places by tensor name
ov::frontend::Place::Ptr get_place_by_tensor_name(const std::string& tensorName) const override;

Same goes for similar constructs in this header.

@mvafin
Apply review feedback
1691f7f 
Signed-off-by: Maxim Vafin <[email protected]>
@mvafin
Copy link
Contributor Author

mvafin commented Oct 11, 2025

Applied some feedback, will do the required improvements in next PRs

@mvafin mvafin enabled auto-merge October 11, 2025 22:06
@mvafin mvafin added this pull request to the merge queue Oct 12, 2025
Merged via the queue into openvinotoolkit:master with commit 32006c1 Oct 12, 2025
207 checks passed
@mvafin mvafin deleted the mvafin/onnx/delegate branch October 12, 2025 10:15
@susbhere susbhere mentioned this pull request Oct 13, 2025
Closed
susbhere pushed a commit to susbhere/openvino that referenced this pull request Oct 13, 2025
@mvafin @susbhere
[OV] Optimze copy tensor with padding
197003d 
Tensor layout related properties are calculated once and used those
cached values during per element offset calculation.

[ONNX] Introduce GraphIterator interface in frontend (openvinotoolkit#32325)

 - *Introduce GraphIterator for ONNX*

 - *CVS-156050*

---------

Signed-off-by: Maxim Vafin <[email protected]>
@mvafin mvafin mentioned this pull request Oct 13, 2025
Closed
@mvafin mvafin added this to the 2025.4 milestone Oct 13, 2025
susbhere pushed a commit to susbhere/openvino that referenced this pull request Oct 14, 2025
@mvafin @susbhere
[OV] Optimze copy tensor with padding
166a060 
Tensor layout related properties are calculated once and used those
cached values during per element offset calculation.

[ONNX] Introduce GraphIterator interface in frontend (openvinotoolkit#32325)

 - *Introduce GraphIterator for ONNX*

 - *CVS-156050*

---------

Signed-off-by: Maxim Vafin <[email protected]>
susbhere pushed a commit to susbhere/openvino that referenced this pull request Oct 15, 2025
@mvafin @susbhere
[OV] Optimze copy tensor with padding
a3f8126 
Tensor layout related properties are calculated once and used those
cached values during per element offset calculation.

[ONNX] Introduce GraphIterator interface in frontend (openvinotoolkit#32325)

 - *Introduce GraphIterator for ONNX*

 - *CVS-156050*

---------

Signed-off-by: Maxim Vafin <[email protected]>
susbhere pushed a commit to susbhere/openvino that referenced this pull request Oct 15, 2025
@mvafin @susbhere
[OV] Optimze copy tensor with padding
4c4ce64 
Tensor layout related properties are calculated once and used those
cached values during per element offset calculation.

[ONNX] Introduce GraphIterator interface in frontend (openvinotoolkit#32325)

 - *Introduce GraphIterator for ONNX*

 - *CVS-156050*

---------

Signed-off-by: Maxim Vafin <[email protected]>
susbhere pushed a commit to susbhere/openvino that referenced this pull request Oct 16, 2025
@mvafin @susbhere
[OV] Optimze copy tensor with padding
bee5498 
Tensor layout related properties are calculated once and used those
cached values during per element offset calculation.

[ONNX] Introduce GraphIterator interface in frontend (openvinotoolkit#32325)

 - *Introduce GraphIterator for ONNX*

 - *CVS-156050*

---------

Signed-off-by: Maxim Vafin <[email protected]>
susbhere pushed a commit to susbhere/openvino that referenced this pull request Oct 16, 2025
@mvafin @susbhere
[OV] Optimze copy tensor with padding
35647e0 
Tensor layout related properties are calculated once and used those
cached values during per element offset calculation.

[ONNX] Introduce GraphIterator interface in frontend (openvinotoolkit#32325)

 - *Introduce GraphIterator for ONNX*

 - *CVS-156050*

---------

Signed-off-by: Maxim Vafin <[email protected]>
susbhere pushed a commit to susbhere/openvino that referenced this pull request Oct 16, 2025
@mvafin @susbhere
[OV] Optimze copy tensor with padding
c438409 
Tensor layout related properties are calculated once and used those
cached values during per element offset calculation.

[ONNX] Introduce GraphIterator interface in frontend (openvinotoolkit#32325)

 - *Introduce GraphIterator for ONNX*

 - *CVS-156050*

---------

Signed-off-by: Maxim Vafin <[email protected]>
susbhere pushed a commit to susbhere/openvino that referenced this pull request Oct 17, 2025
@mvafin @susbhere
[OV] Optimze copy tensor with padding
53a6922 
Tensor layout related properties are calculated once and used those
cached values during per element offset calculation.

[ONNX] Introduce GraphIterator interface in frontend (openvinotoolkit#32325)

 - *Introduce GraphIterator for ONNX*

 - *CVS-156050*

---------

Signed-off-by: Maxim Vafin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rkazants rkazants rkazants left review comments

@bumbosiepsak bumbosiepsak bumbosiepsak approved these changes

Assignees

No one assigned

Labels

category: build OpenVINO cmake script / infra category: CPP API OpenVINO CPP API bindings category: ONNX FE OpenVINO ONNX FrontEnd

Projects

None yet

Milestone

2025.4

Development

Successfully merging this pull request may close these issues.

3 participants

@mvafin @bumbosiepsak @rkazants