[ONNX] Added experimental support of GraphIterator interface #31935
Conversation
github-actions
bot
added
category: build
gkrivor
force-pushed
the
onnx_gi
branch
2 times, most recently
from
be2b917 to
09fe2e3
Compare
gkrivor
changed the title
gkrivor
force-pushed
the
onnx_gi
branch
2 times, most recently
from
2971cec to
cfcf503
Compare
src/frontends/onnx/frontend/include/openvino/frontend/onnx/decoder.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
Adds experimental GraphIterator-based loading/decoding path to the ONNX FrontEnd to allow iterating external or in‑memory ONNX graphs without relying solely on protobuf Graph parsing. Key changes introduce new iterator/decoder abstractions, translation session logic, and adapt control‑flow & constant ops to work with both legacy and iterator modes.
- Introduces GraphIterator / Decoder abstractions with proto-based implementation (graph_iterator_proto, decoder_proto) and unified InputModel (unify::InputModel).
- Adds TranslateSession and updates op translators (Loop, If, Scan, Constant, Unsqueeze) to branch on iterator-based decoding.
- Extends tests and build scripts to exercise new iterator path; updates error expectation and model fixtures.
Reviewed Changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| onnx_import_controlflow.in.cpp | Adjusted expected error substrings in a negative test. |
| bool_init_raw.prototxt | Added Identity node and changed model output to new tensor. |
| graph_iterator.cpp (test) | New tests for simple and external GraphIterator usage. |
| CMakeLists.txt | Includes new source files (graph iterator & decoder) in test target. |
| tensor_external_data.* | Added constructor and relaxed path handling for external data. |
| pooling_factory.* | Refactored member m_onnx_node from value to pointer. |
| translate_session.* | Introduces TranslateSession orchestration and graph translation logic. |
| place.hpp / place.cpp (core) | Added unified Place hierarchy for iterator mode. |
| ops_bridge.* | Added direct operator lookup by domain/opset. |
| op/{unsqueeze,scan,loop,if,constant}.cpp | Added iterator-mode branches and behavior adjustments. |
| input_model.* | Added unify::InputModel implementation for GraphIterator path. |
| frontend.cpp / frontend.hpp | Integrated iterator-mode load/convert/decode paths. |
| core/* (tensor, node, graph_iterator_proto, decoder_proto, etc.) | Core iterator & decoder implementations plus node adaptation. |
| decoder.hpp / graph_iterator.hpp | New public ONNX frontend iterator/decoder interfaces. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (!error_message.empty()) { | ||
| auto telemetry = model_onnx->get_telemetry_extension(); | ||
| if (m_fail_fast) { | ||
| if (telemetry && translator == nullptr) { | ||
| telemetry->send_event("error_cause", "onnx_" + decoder->get_op_type()); | ||
| } | ||
| throw; | ||
| } else { |
There was a problem hiding this comment.
throw; is executed outside the dynamic scope of a catch block (the catch blocks have already completed), leading to undefined behavior and likely std::terminate. Store and rethrow the caught exception (e.g. rethrow the original inside the catch) or throw a new exception (e.g. throw std::runtime_error(error_message);).
Copilot uses AI. Check for mistakes.
| ov::Any Node::get_attribute_any(const std::string& name) const { | ||
| if (m_pimpl != nullptr) { | ||
| } else if (m_decoder != nullptr) { | ||
| return m_decoder->get_attribute(name); | ||
| } | ||
| return *found_attr; | ||
| FRONT_END_NOT_IMPLEMENTED(__FUNCTION__); |
There was a problem hiding this comment.
When m_pimpl is non-null the function falls through without returning, triggering the NOT_IMPLEMENTED path and never returning the attribute. Implement returning the stored attribute for the legacy (m_pimpl) path (e.g. return get_attribute(name).get_any();).
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
|
These changes were merged as #32325 |
4 participants
Details:
Tickets: