Copy outer-scope initializers used only from a subgraph when compiling - #32158
Copy outer-scope initializers used only from a subgraph when compiling#32158hcl (hclsys) wants to merge 1 commit into
Conversation
CreateEpContextModel copied an initializer into the EPContext graph only when a NodeArg of that name already existed. ep_graph.AddNode() creates NodeArgs for a node's own inputs and outputs but not for names its subgraphs consume from outer scope (implicit inputs), so an initializer read only from inside an If/Loop/Scan body was skipped and the following ep_graph.Resolve() failed on the dangling reference. Such models load and run correctly in a normal session; only the compile API rejected them. Also copy initializers named by a node's ImplicitInputDefs(). The copy helper creates the NodeArg for the name, so the outer-scope reference resolves. Fixes microsoft#32131 Signed-off-by: Chenglun Hu <chenglunhu@gmail.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
hcl (@hclsys) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes EPContext model creation so outer-scope initializers referenced only by subgraphs (implicit inputs) are still copied, preventing resolve/compile failures; adds a regression test for this scenario.
Changes:
- Add a Python regression test that builds an
Ifmodel whose branch bodies reference outer-scope initializers. - Update EPContext model construction to also copy initializers that are referenced as implicit inputs in subgraphs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| onnxruntime/test/python/onnxruntime_test_python_compile_api.py | Adds a compile regression test where subgraphs consume only outer-scope initializers. |
| onnxruntime/core/framework/graph_partitioner.cc | Ensures initializers referenced via implicit inputs are copied into the EPContext model to avoid dangling references. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| InlinedHashSet<std::string_view> implicit_input_names; | ||
| for (const auto& node : graph.Nodes()) { | ||
| for (const auto* implicit_input : node.ImplicitInputDefs()) { | ||
| if (implicit_input != nullptr && implicit_input->Exists()) { | ||
| implicit_input_names.insert(implicit_input->Name()); | ||
| } | ||
| } | ||
| } |
| // handle initializers | ||
| for (const auto& [name, _] : graph.GetAllInitializedTensors()) { | ||
| if (ep_graph.GetNodeArg(name) != nullptr) { | ||
| if (ep_graph.GetNodeArg(name) != nullptr || implicit_input_names.count(name) != 0) { |
| [axes, data, cond], | ||
| ) | ||
| model = onnx.helper.make_model(graph, opset_imports=[onnx.helper.make_opsetid("", 21)]) | ||
| model.ir_version = 10 |
|
On the copilot note about scanning |
Fixes #32131.
CompileModel/OrtCompileApifails on any model where a subgraph (If/Loop/Scanbody) reads an initializer of an enclosing graph that no node of the enclosing graph reads itself — which the ONNX IR spec allows, and whichInferenceSessionandoptimized_model_filepathhandle fine.CreateEpContextModel(graph_partitioner.cc) copies an initializer into the EPContext graph only when aNodeArgof that name already exists.ep_graph.AddNode()createsNodeArgs for a node's own inputs/outputs but not for names its subgraphs consume from outer scope (implicit inputs), so a subgraph-only initializer is skipped and the followingep_graph.Resolve()fails on the dangling reference.Fix: also copy initializers named by a node's
ImplicitInputDefs().MakeInitializerCopyIfNotExistcreates theNodeArgfor the copied initializer, so the outer-scope reference resolves.Verified locally (built with the change): a minimal
Ifmodel whose branches read outer-scopeaxes/datano outer node reads now compiles (ModelCompiler.compile_to_bytessucceeds) where it previously raisedINVALID_GRAPH. Added a regression testtest_compile_model_with_outer_scope_initializer_used_only_in_subgraphtoonnxruntime_test_python_compile_api.py.