From 88edd5f8470ab74b67063c5ba07de843760e8d6a Mon Sep 17 00:00:00 2001 From: Frederic Devernay Date: Fri, 1 May 2026 10:37:59 +0200 Subject: [PATCH 1/3] Fix ModelEditorValueInfoToOnnx dropping symbolic dim names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ModelEditorValueInfoToOnnx had a missing increment bug: `size_t idx = 0;` was declared before the dim loop but never incremented, so `dim_params[idx]` always read the first entry. Every dynamic dimension in the generated ValueInfoProto was assigned the same name — the name of the first dim — instead of its own. For a ValueInfo built with shape `[-1, -1, -1]` and symbolic dim names `["N", "H", "W"]`, the resulting session exposed dims `["N", "N", "N"]` via `SessionGetInputTypeInfo` → `GetSymbolicDimensions`. Downstream, this causes the TensorRT EP to reject valid dynamic shape profiles. When `trt_profile_{min,opt,max}_shapes` bind distinct ranges to each dim, TRT (correctly) sees all three ranges attached to the same symbolic name and errors with: "N: Input dimensions with this name have different constant values or have contradictory IOptimizationProfile kMIN/kMAX constraints." The fix indexes dim_params by the dim position, restoring the invariant that `dim_params[i]` corresponds to `dims[i]`, consistent with: - OrtApis::SetSymbolicDimensions (stores names by position) - OrtApis::GetSymbolicDimensions (reads names by position) - OrtTypeInfo::FromTypeProto (reconstructs by position) The doc comment on Basic_CApi already documents the expected behavior: "call SetDimensions with {-1, 3, 2} and SetSymbolicDimensions with {\"N\", nullptr, nullptr} to create a shape of {\"N\", 3, 2}". Adds a regression test SymbolicDimensions_DistinctNamesPreserved that builds a minimal Identity model via the C++ Model Editor API with three named dynamic dims, finalizes the session, and asserts the names round-trip correctly via SessionGetInputTypeInfo. The existing Basic_CxxApi test happens to use only one non-empty symbolic name ("multiple_of_3", empty on the other dim), which masked this bug: with only one name, always reading dim_params[0] happens to produce the correct result. --- onnxruntime/core/graph/graph.cc | 4 +- .../test/shared_lib/test_model_builder_api.cc | 71 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/onnxruntime/core/graph/graph.cc b/onnxruntime/core/graph/graph.cc index 1f05dfd38968a..82bf3c13d5bf0 100644 --- a/onnxruntime/core/graph/graph.cc +++ b/onnxruntime/core/graph/graph.cc @@ -6693,10 +6693,10 @@ ValueInfoProto ModelEditorValueInfoToOnnx(const onnxruntime::ModelEditorValueInf if (tensor_info.HasShape()) { auto& shape = *tensor->mutable_shape(); - size_t idx = 0; const auto dims = tensor_info.GetShape()->GetDims(); const auto& dim_params = *tensor_info.GetDimParams(); - for (auto dim : dims) { + for (size_t idx = 0; idx < dims.size(); ++idx) { + const auto dim = dims[idx]; auto& dim_proto = *shape.add_dim(); if (dim >= 0) { dim_proto.set_dim_value(dim); diff --git a/onnxruntime/test/shared_lib/test_model_builder_api.cc b/onnxruntime/test/shared_lib/test_model_builder_api.cc index c5ec376f7d0f5..ad50117e4bd19 100644 --- a/onnxruntime/test/shared_lib/test_model_builder_api.cc +++ b/onnxruntime/test/shared_lib/test_model_builder_api.cc @@ -728,6 +728,77 @@ TEST(ModelEditorAPITest, CreateTypeInfo) { api.ReleaseTypeInfo(base_tensor_type_info); } +// Regression test for a bug in ModelEditorValueInfoToOnnx (core/graph/graph.cc) +// where all dynamic dimensions of a graph input were assigned the same name +// (the first one in dim_params) instead of preserving each dim's individual name. +// +// The bug: inside ModelEditorValueInfoToOnnx, `size_t idx = 0;` was declared +// before the loop but never incremented, so `dim_params[idx]` always read +// `dim_params[0]`. +// +// Repro: create a graph input with three dynamic dimensions named "N", "H", "W". +// Before the fix, GetInputTypeInfo after finalizing the session would return +// `["N", "N", "N"]` instead of `["N", "H", "W"]`. +// +// This also triggers a downstream TensorRT EP failure when the user supplies +// a dynamic shape profile (trt_profile_{min,opt,max}_shapes) — TRT sees dim 0 +// bounded to [1..16] and dim 1 bounded to [192..2160] *under the same +// name "N"* and errors with: +// "N: Input dimensions with this name have different constant values or +// have contradictory IOptimizationProfile kMIN/kMAX constraints." +TEST(ModelEditorAPITest, SymbolicDimensions_DistinctNamesPreserved) { + // Build a minimal model: input x -> Identity -> output y, where x has + // 3 dynamic dims named {"N", "H", "W"}. + Ort::Graph graph; + std::vector graph_inputs; + std::vector graph_outputs; + + std::vector dims = {-1, -1, -1}; + std::vector sym = {"N", "H", "W"}; + + TensorTypeAndShapeInfo input_info(ONNXTensorElementDataType::ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT, dims, &sym); + auto input_type_info = TypeInfo::CreateTensorInfo(input_info.GetConst()); + graph_inputs.emplace_back("x", input_type_info.GetConst()); + + TensorTypeAndShapeInfo output_info(ONNXTensorElementDataType::ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT, dims, &sym); + auto output_type_info = TypeInfo::CreateTensorInfo(output_info.GetConst()); + graph_outputs.emplace_back("y", output_type_info.GetConst()); + + graph.SetInputs(graph_inputs); + graph.SetOutputs(graph_outputs); + + Ort::Node identity("Identity", onnxruntime::kOnnxDomain, "id", {"x"}, {"y"}); + graph.AddNode(identity); + + std::vector opsets{{onnxruntime::kOnnxDomain, 21}}; + Model model(opsets); + model.AddGraph(graph); + + auto session = CreateSession(*ort_env, model); + + // Query back the input type info. + auto queried_type_info = session.GetInputTypeInfo(0); + auto queried_tensor_info = queried_type_info.GetTensorTypeAndShapeInfo(); + + auto queried_dims = queried_tensor_info.GetShape(); + ASSERT_EQ(queried_dims.size(), 3u) << "Expected 3 dimensions"; + EXPECT_EQ(queried_dims[0], -1); + EXPECT_EQ(queried_dims[1], -1); + EXPECT_EQ(queried_dims[2], -1); + + auto queried_sym = queried_tensor_info.GetSymbolicDimensions(); + ASSERT_EQ(queried_sym.size(), 3u) << "Expected 3 symbolic dim names"; + ASSERT_NE(queried_sym[0], nullptr); + ASSERT_NE(queried_sym[1], nullptr); + ASSERT_NE(queried_sym[2], nullptr); + + // Each dim's symbolic name must match what we set. The bug caused all + // three names to equal queried_sym[0] ("N"). + EXPECT_STREQ(queried_sym[0], "N") << "dim 0 name must be preserved"; + EXPECT_STREQ(queried_sym[1], "H") << "dim 1 name must be preserved (bug would yield \"N\")"; + EXPECT_STREQ(queried_sym[2], "W") << "dim 2 name must be preserved (bug would yield \"N\")"; +} + // // Tests for Model Editor API + Compile API integration // From 0881d46610deef1bcfb79d0e3e73ae2f967d455b Mon Sep 17 00:00:00 2001 From: Frederic Devernay Date: Fri, 1 May 2026 10:37:59 +0200 Subject: [PATCH 2/3] test: add partial-naming case for symbolic dim regression Adds ModelEditorAPITest.SymbolicDimensions_PartiallyNamedDimensions which covers the case where only some dimensions carry a symbolic name and others are either static or dynamic-but-unnamed. Matches the example given in the Basic_CApi test's doc comment: "call SetDimensions with {-1, 3, 2} and SetSymbolicDimensions with {\"N\", nullptr, nullptr} to create a shape of {\"N\", 3, 2}" --- .../test/shared_lib/test_model_builder_api.cc | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/onnxruntime/test/shared_lib/test_model_builder_api.cc b/onnxruntime/test/shared_lib/test_model_builder_api.cc index ad50117e4bd19..ddbf1ae79e1e7 100644 --- a/onnxruntime/test/shared_lib/test_model_builder_api.cc +++ b/onnxruntime/test/shared_lib/test_model_builder_api.cc @@ -799,6 +799,68 @@ TEST(ModelEditorAPITest, SymbolicDimensions_DistinctNamesPreserved) { EXPECT_STREQ(queried_sym[2], "W") << "dim 2 name must be preserved (bug would yield \"N\")"; } +// Regression test: partially-named symbolic dimensions. Covers the case where +// only some dimensions carry a symbolic name and others are unnamed (or static). +// This is the exact example given in the Basic_CApi test's doc comment: +// "call SetDimensions with {-1, 3, 2} and SetSymbolicDimensions with +// {\"N\", nullptr, nullptr} to create a shape of {\"N\", 3, 2}" +// The C++ API wrapper represents "no name" as an empty string. +TEST(ModelEditorAPITest, SymbolicDimensions_PartiallyNamedDimensions) { + // Shape: [N, 3, -1] + // dim 0 = dynamic, named "N" + // dim 1 = static, size 3 + // dim 2 = dynamic, unnamed + Ort::Graph graph; + std::vector graph_inputs; + std::vector graph_outputs; + + std::vector dims = {-1, 3, -1}; + std::vector sym = {"N", "", ""}; // static and unnamed dims get empty strings + + TensorTypeAndShapeInfo input_info(ONNXTensorElementDataType::ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT, dims, &sym); + auto input_type_info = TypeInfo::CreateTensorInfo(input_info.GetConst()); + graph_inputs.emplace_back("x", input_type_info.GetConst()); + + TensorTypeAndShapeInfo output_info(ONNXTensorElementDataType::ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT, dims, &sym); + auto output_type_info = TypeInfo::CreateTensorInfo(output_info.GetConst()); + graph_outputs.emplace_back("y", output_type_info.GetConst()); + + graph.SetInputs(graph_inputs); + graph.SetOutputs(graph_outputs); + + Ort::Node identity("Identity", onnxruntime::kOnnxDomain, "id", {"x"}, {"y"}); + graph.AddNode(identity); + + std::vector opsets{{onnxruntime::kOnnxDomain, 21}}; + Model model(opsets); + model.AddGraph(graph); + + auto session = CreateSession(*ort_env, model); + + auto queried_type_info = session.GetInputTypeInfo(0); + auto queried_tensor_info = queried_type_info.GetTensorTypeAndShapeInfo(); + + auto queried_dims = queried_tensor_info.GetShape(); + ASSERT_EQ(queried_dims.size(), 3u); + EXPECT_EQ(queried_dims[0], -1); + EXPECT_EQ(queried_dims[1], 3); // static + EXPECT_EQ(queried_dims[2], -1); + + auto queried_sym = queried_tensor_info.GetSymbolicDimensions(); + ASSERT_EQ(queried_sym.size(), 3u); + + // Dim 0 retains its symbolic name. + ASSERT_NE(queried_sym[0], nullptr); + EXPECT_STREQ(queried_sym[0], "N") << "dim 0 name must be preserved"; + // Dim 1 is static; dim 2 is dynamic but unnamed. In both cases, the + // corresponding dim_proto should not have a dim_param set, so the C API + // returns an empty string. + ASSERT_NE(queried_sym[1], nullptr); + EXPECT_STREQ(queried_sym[1], "") << "dim 1 is static; no dim_param expected"; + ASSERT_NE(queried_sym[2], nullptr); + EXPECT_STREQ(queried_sym[2], "") << "dim 2 is dynamic but unnamed; no dim_param expected"; +} + // // Tests for Model Editor API + Compile API integration // From f359ec6be31da84ff47041044c511c6578322331 Mon Sep 17 00:00:00 2001 From: Frederic Devernay Date: Fri, 1 May 2026 10:42:00 +0200 Subject: [PATCH 3/3] test: add C API nullptr case for SetSymbolicDimensions Adds ModelEditorAPITest.SymbolicDimensions_CApi_NullNames which mirrors the literal example from the Basic_CApi test's doc comment: 'call SetDimensions with {-1, 3, 2} and SetSymbolicDimensions with {"N", nullptr, nullptr} to create a shape of {"N", 3, 2}' The existing C++ TensorTypeAndShapeInfo wrapper takes std::vector* and represents 'no name' as an empty string, so the C++ tests cannot exercise the nullptr path directly. This test drops to the raw C API to cover the nullptr case. Also updates the partial-naming test's comment to reference this companion test and clarify the empty-string-vs-nullptr distinction. --- .../test/shared_lib/test_model_builder_api.cc | 118 +++++++++++++++++- 1 file changed, 117 insertions(+), 1 deletion(-) diff --git a/onnxruntime/test/shared_lib/test_model_builder_api.cc b/onnxruntime/test/shared_lib/test_model_builder_api.cc index ddbf1ae79e1e7..bf70755882d05 100644 --- a/onnxruntime/test/shared_lib/test_model_builder_api.cc +++ b/onnxruntime/test/shared_lib/test_model_builder_api.cc @@ -804,7 +804,10 @@ TEST(ModelEditorAPITest, SymbolicDimensions_DistinctNamesPreserved) { // This is the exact example given in the Basic_CApi test's doc comment: // "call SetDimensions with {-1, 3, 2} and SetSymbolicDimensions with // {\"N\", nullptr, nullptr} to create a shape of {\"N\", 3, 2}" -// The C++ API wrapper represents "no name" as an empty string. +// This test uses the C++ wrapper, which takes `std::vector*` and +// therefore represents "no name" as an empty string. A companion test below +// (SymbolicDimensions_CApi_NullNames) exercises the raw C API with nullptr +// entries to cover the nullptr path documented in Basic_CApi's comment. TEST(ModelEditorAPITest, SymbolicDimensions_PartiallyNamedDimensions) { // Shape: [N, 3, -1] // dim 0 = dynamic, named "N" @@ -861,6 +864,119 @@ TEST(ModelEditorAPITest, SymbolicDimensions_PartiallyNamedDimensions) { EXPECT_STREQ(queried_sym[2], "") << "dim 2 is dynamic but unnamed; no dim_param expected"; } +// Regression test: raw C API with nullptr entries in SetSymbolicDimensions. +// Covers two things the other tests don't: +// 1. That multiple distinct symbolic names on different dynamic dims all +// round-trip correctly through the raw C API (this path also exhibited +// the bug before the fix). +// 2. The nullptr-for-unnamed-dim convention from the Basic_CApi doc comment: +// "call SetDimensions with {-1, 3, 2} and SetSymbolicDimensions with +// {\"N\", nullptr, nullptr} to create a shape of {\"N\", 3, 2}" +// The C++ TensorTypeAndShapeInfo wrapper takes std::vector* and +// cannot pass nullptr entries, so this test drops to the raw C API. +TEST(ModelEditorAPITest, SymbolicDimensions_CApi_NullNames) { + const auto& api = Ort::GetApi(); + const auto& model_editor_api = Ort::GetModelEditorApi(); + + // Shape: [N, 3, H, W] + // dim 0 = dynamic, named "N" + // dim 1 = static, size 3 (nullptr in names list to match the doc example) + // dim 2 = dynamic, named "H" + // dim 3 = dynamic, named "W" + // Before the fix, dims 2 and 3 would both come back as "N" (always + // reading dim_params[0]). + OrtTensorTypeAndShapeInfo* input_tensor_info = nullptr; + Ort::ThrowOnError(api.CreateTensorTypeAndShapeInfo(&input_tensor_info)); + Ort::ThrowOnError(api.SetTensorElementType(input_tensor_info, ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT)); + std::vector dims = {-1, 3, -1, -1}; + Ort::ThrowOnError(api.SetDimensions(input_tensor_info, dims.data(), dims.size())); + std::vector sym = {"N", nullptr, "H", "W"}; + Ort::ThrowOnError(api.SetSymbolicDimensions(input_tensor_info, sym.data(), sym.size())); + + OrtTypeInfo* input_type_info = nullptr; + Ort::ThrowOnError(model_editor_api.CreateTensorTypeInfo(input_tensor_info, &input_type_info)); + api.ReleaseTensorTypeAndShapeInfo(input_tensor_info); + + OrtValueInfo* input_value_info = nullptr; + Ort::ThrowOnError(model_editor_api.CreateValueInfo("x", input_type_info, &input_value_info)); + api.ReleaseTypeInfo(input_type_info); + + // Output has the same shape (Identity op). + OrtTensorTypeAndShapeInfo* output_tensor_info = nullptr; + Ort::ThrowOnError(api.CreateTensorTypeAndShapeInfo(&output_tensor_info)); + Ort::ThrowOnError(api.SetTensorElementType(output_tensor_info, ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT)); + Ort::ThrowOnError(api.SetDimensions(output_tensor_info, dims.data(), dims.size())); + Ort::ThrowOnError(api.SetSymbolicDimensions(output_tensor_info, sym.data(), sym.size())); + + OrtTypeInfo* output_type_info = nullptr; + Ort::ThrowOnError(model_editor_api.CreateTensorTypeInfo(output_tensor_info, &output_type_info)); + api.ReleaseTensorTypeAndShapeInfo(output_tensor_info); + + OrtValueInfo* output_value_info = nullptr; + Ort::ThrowOnError(model_editor_api.CreateValueInfo("y", output_type_info, &output_value_info)); + api.ReleaseTypeInfo(output_type_info); + + OrtGraph* graph = nullptr; + Ort::ThrowOnError(model_editor_api.CreateGraph(&graph)); + std::vector graph_inputs = {input_value_info}; + std::vector graph_outputs = {output_value_info}; + Ort::ThrowOnError(model_editor_api.SetGraphInputs(graph, graph_inputs.data(), graph_inputs.size())); + Ort::ThrowOnError(model_editor_api.SetGraphOutputs(graph, graph_outputs.data(), graph_outputs.size())); + + std::vector node_inputs = {"x"}; + std::vector node_outputs = {"y"}; + OrtNode* identity_node = nullptr; + Ort::ThrowOnError(model_editor_api.CreateNode("Identity", onnxruntime::kOnnxDomain, "id", + node_inputs.data(), node_inputs.size(), + node_outputs.data(), node_outputs.size(), + nullptr, 0, &identity_node)); + Ort::ThrowOnError(model_editor_api.AddNodeToGraph(graph, identity_node)); + + std::vector opset_domains = {onnxruntime::kOnnxDomain}; + std::vector opset_versions = {21}; + OrtModel* model = nullptr; + Ort::ThrowOnError(model_editor_api.CreateModel(opset_domains.data(), opset_versions.data(), + opset_domains.size(), &model)); + Ort::ThrowOnError(model_editor_api.AddGraphToModel(model, graph)); + + Ort::SessionOptions session_options; + OrtSession* session = nullptr; + Ort::ThrowOnError(model_editor_api.CreateSessionFromModel(*ort_env, model, session_options, &session)); + api.ReleaseModel(model); + + // Query back and verify each dim. + OrtTypeInfo* queried_type_info = nullptr; + Ort::ThrowOnError(api.SessionGetInputTypeInfo(session, 0, &queried_type_info)); + + const OrtTensorTypeAndShapeInfo* queried_tensor_info = nullptr; + Ort::ThrowOnError(api.CastTypeInfoToTensorInfo(queried_type_info, &queried_tensor_info)); + + size_t dim_count = 0; + Ort::ThrowOnError(api.GetDimensionsCount(queried_tensor_info, &dim_count)); + ASSERT_EQ(dim_count, 4u); + + std::vector queried_dims(dim_count, -42); + Ort::ThrowOnError(api.GetDimensions(queried_tensor_info, queried_dims.data(), queried_dims.size())); + EXPECT_EQ(queried_dims[0], -1); + EXPECT_EQ(queried_dims[1], 3); + EXPECT_EQ(queried_dims[2], -1); + EXPECT_EQ(queried_dims[3], -1); + + std::vector queried_sym(dim_count, nullptr); + Ort::ThrowOnError(api.GetSymbolicDimensions(queried_tensor_info, queried_sym.data(), queried_sym.size())); + ASSERT_NE(queried_sym[0], nullptr); + EXPECT_STREQ(queried_sym[0], "N") << "dim 0 keeps its symbolic name"; + ASSERT_NE(queried_sym[1], nullptr); + EXPECT_STREQ(queried_sym[1], "") << "dim 1 is static (nullptr name); no dim_param expected"; + ASSERT_NE(queried_sym[2], nullptr); + EXPECT_STREQ(queried_sym[2], "H") << "dim 2 name must be preserved (bug would yield \"N\")"; + ASSERT_NE(queried_sym[3], nullptr); + EXPECT_STREQ(queried_sym[3], "W") << "dim 3 name must be preserved (bug would yield \"N\")"; + + api.ReleaseTypeInfo(queried_type_info); + api.ReleaseSession(session); +} + // // Tests for Model Editor API + Compile API integration //