From f85a662563016070c0bf484ead086e2105bf05e3 Mon Sep 17 00:00:00 2001 From: Abhishek Gola Date: Tue, 19 May 2026 15:08:16 +0530 Subject: [PATCH] Merge pull request #28971 from abhishek-gola:subgraph_argname_fix Fixed subgraph name scoping in new DNN engine #28971 closes: https://github.com/opencv/opencv/issues/23663, https://github.com/opencv/opencv/issues/19977 OpenCV extra: https://github.com/opencv/opencv_extra/pull/1362 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake --- modules/dnn/src/graph_buffer_allocator.cpp | 40 ++++++ modules/dnn/src/onnx/onnx_importer2.cpp | 150 +++++++++++++++++++-- modules/dnn/test/test_layers.cpp | 45 +++++++ 3 files changed, 223 insertions(+), 12 deletions(-) diff --git a/modules/dnn/src/graph_buffer_allocator.cpp b/modules/dnn/src/graph_buffer_allocator.cpp index 51ef22c7c6..6563ddc084 100644 --- a/modules/dnn/src/graph_buffer_allocator.cpp +++ b/modules/dnn/src/graph_buffer_allocator.cpp @@ -5,6 +5,8 @@ #include "precomp.hpp" #include "net_impl.hpp" +#include + namespace cv { namespace dnn { CV__DNN_INLINE_NS_BEGIN @@ -353,10 +355,48 @@ struct BufferAllocator } } + // The body reads names produced in the enclosing scope + // (closure references) without listing them in the Loop/If + // layer's inputs. Bump the outer-scope usecount of each + // such arg so its buffer survives until the subgraph runs. + std::unordered_set bodyDefined; + for (Arg ba : body->inputs()) + bodyDefined.insert(ba.idx); + for (const Ptr& blayer : body->prog()) { + if (!blayer) continue; + for (Arg bo : blayer->outputs) + bodyDefined.insert(bo.idx); + } + std::unordered_set closureBumped; + for (const Ptr& blayer : body->prog()) { + if (!blayer) continue; + for (Arg bi : blayer->inputs) { + if (bi.idx <= 0) continue; + if (bodyDefined.count(bi.idx)) continue; + if (netimpl->isConstArg(bi)) continue; + if (bufidxs[bi.idx] < 0) continue; + if (closureBumped.insert(bi.idx).second) { + usecounts[bi.idx]++; + buf_usecounts[bufidxs[bi.idx]]++; + } + } + } + std::vector saved_freebufs = freebufs; freebufs.clear(); assign(body); freebufs = saved_freebufs; + + for (int idx : closureBumped) { + int bidx = bufidxs[idx]; + if (--usecounts[idx] == 0) { + if (bidx >= 0) + releaseBuffer(bidx); + } else if (bidx >= 0) { + CV_Assert(buf_usecounts[bidx] > 0); + --buf_usecounts[bidx]; + } + } } for (auto out: outputs) { diff --git a/modules/dnn/src/onnx/onnx_importer2.cpp b/modules/dnn/src/onnx/onnx_importer2.cpp index 5cf224acc5..438370f5fd 100644 --- a/modules/dnn/src/onnx/onnx_importer2.cpp +++ b/modules/dnn/src/onnx/onnx_importer2.cpp @@ -22,9 +22,11 @@ #include #include #include +#include #include #include #include +#include #if defined _MSC_VER && _MSC_VER < 1910/*MSVS 2017*/ #pragma warning(push) @@ -131,6 +133,23 @@ protected: Mat parseTensor(const opencv_onnx::TensorProto& tensorProto); void rememberMissingOp(const std::string& opname); + // Subgraph body-local names are renamed to "" so + // they cannot shadow the enclosing scope. + struct RenameUndo { + std::string key; + bool had_prev; + std::string prev_value; + }; + std::string remap(const std::string& name) const; + void pushSubgraphRenames(const opencv_onnx::GraphProto& graph_proto, + std::vector& undos); + void sanitizeMainGraphNames(const opencv_onnx::GraphProto& graph_proto); + void recordSubgraphRename(const std::string& raw_name, + const std::string& prefix, + std::vector& undos); + void sanitizeName(const std::string& raw_name); + void popRenames(const std::vector& undos); + LayerParams getLayerParams(const opencv_onnx::NodeProto& node_proto); void addLayer(LayerParams& layerParams, @@ -157,6 +176,8 @@ protected: // Used when Onnx does not contain node names. // In this case each node is assigned a name 'onnx_node!' int global_node_idx; + std::unordered_map rename_map; + int subgraph_scope_counter; bool have_errors; typedef void (ONNXImporter2::*ONNXImporterNodeParser)(LayerParams& layerParams, const opencv_onnx::NodeProto& node_proto); @@ -661,6 +682,8 @@ void ONNXImporter2::parseOperatorSet() Net ONNXImporter2::parseModel() { global_node_idx = 0; + subgraph_scope_counter = 0; + rename_map.clear(); have_errors = false; CV_Assert(model_proto.has_graph()); opencv_onnx::GraphProto* graph_proto = model_proto.mutable_graph(); @@ -771,6 +794,92 @@ Mat ONNXImporter2::parseTensor(const opencv_onnx::TensorProto& tensor_proto) return getMatFromTensor2(tensor_proto, onnxBasePath); } +std::string ONNXImporter2::remap(const std::string& name) const +{ + if (name.empty()) + return name; + auto it = rename_map.find(name); + return it == rename_map.end() ? name : it->second; +} + +void ONNXImporter2::recordSubgraphRename(const std::string& raw_name, + const std::string& prefix, + std::vector& undos) +{ + if (raw_name.empty()) + return; + RenameUndo u; + u.key = raw_name; + auto it = rename_map.find(raw_name); + u.had_prev = (it != rename_map.end()); + if (u.had_prev) + u.prev_value = it->second; + undos.push_back(std::move(u)); + rename_map[raw_name] = prefix + raw_name; +} + +void ONNXImporter2::sanitizeName(const std::string& raw_name) +{ + if (raw_name.empty() || raw_name.find('#') == std::string::npos) + return; + if (rename_map.find(raw_name) != rename_map.end()) + return; + std::string sanitized = raw_name; + std::replace(sanitized.begin(), sanitized.end(), '#', '_'); + rename_map[raw_name] = std::move(sanitized); +} + +void ONNXImporter2::pushSubgraphRenames(const opencv_onnx::GraphProto& g, + std::vector& undos) +{ + const std::string& gname = g.name(); + const std::string prefix = (gname.empty() ? std::string("body") : gname) + + std::to_string(subgraph_scope_counter++) + "#"; + + const int n_init = g.initializer_size(); + const int n_in = g.input_size(); + const int n_out = g.output_size(); + const int n_nodes = g.node_size(); + undos.reserve(undos.size() + n_init + n_in + n_out + n_nodes); + + for (int i = 0; i < n_init; ++i) + recordSubgraphRename(g.initializer(i).name(), prefix, undos); + for (int i = 0; i < n_in; ++i) + recordSubgraphRename(g.input(i).name(), prefix, undos); + for (int i = 0; i < n_out; ++i) + recordSubgraphRename(g.output(i).name(), prefix, undos); + for (int i = 0; i < n_nodes; ++i) { + const opencv_onnx::NodeProto& n = g.node(i); + for (int j = 0; j < n.output_size(); ++j) + recordSubgraphRename(n.output(j), prefix, undos); + } +} + +void ONNXImporter2::sanitizeMainGraphNames(const opencv_onnx::GraphProto& g) +{ + for (int i = 0; i < g.initializer_size(); ++i) + sanitizeName(g.initializer(i).name()); + for (int i = 0; i < g.input_size(); ++i) + sanitizeName(g.input(i).name()); + for (int i = 0; i < g.output_size(); ++i) + sanitizeName(g.output(i).name()); + for (int i = 0; i < g.node_size(); ++i) { + const opencv_onnx::NodeProto& n = g.node(i); + for (int j = 0; j < n.output_size(); ++j) + sanitizeName(n.output(j)); + } +} + +void ONNXImporter2::popRenames(const std::vector& undos) +{ + for (auto it = undos.rbegin(); it != undos.rend(); ++it) { + if (it->had_prev) + rename_map[it->key] = it->prev_value; + else + rename_map.erase(it->key); + } +} + Ptr ONNXImporter2::parseGraph(opencv_onnx::GraphProto* graph_proto, bool mainGraph_) { CV_LOG_DEBUG(NULL, "DNN/ONNX: parsing graph '" << graph_proto->name() << "' of " << graph_proto->node_size() << " nodes"); @@ -778,6 +887,12 @@ Ptr ONNXImporter2::parseGraph(opencv_onnx::GraphProto* graph_proto, bool int n_nodes = graph_proto->node_size(); CV_LOG_DEBUG(NULL, "DNN/ONNX: simplified the graph to " << n_nodes << " nodes"); + std::vector local_renames; + if (mainGraph_) + sanitizeMainGraphNames(*graph_proto); + else + pushSubgraphRenames(*graph_proto, local_renames); + opencv_onnx::GraphProto* saved_graph_proto = curr_graph_proto; Ptr saved_graph = curr_graph; std::vector > saved_prog; @@ -788,21 +903,24 @@ Ptr ONNXImporter2::parseGraph(opencv_onnx::GraphProto* graph_proto, bool // parse constant tensors int n_consts = graph_proto->initializer_size(); for (int i = 0; i < n_consts; i++) { - //const opencv_onnx:: const opencv_onnx::TensorProto& const_i = graph_proto->initializer(i); Mat t = parseTensor(const_i); - netimpl->newConstArg(const_i.name(), t); + netimpl->newConstArg(remap(const_i.name()), t); } // parse graph inputs int n_inputs = graph_proto->input_size(); for (int i = 0; i < n_inputs; i++) { const opencv_onnx::ValueInfoProto& input_i = graph_proto->input(i); - if (net.haveArg(input_i.name())) + std::string input_name = remap(input_i.name()); + // ONNX permits an initializer with the same name to shadow an input, + // promoting it to a constant. In that case the input entry is ignored. + if (net.haveArg(input_name)) continue; - Arg arg = netimpl->newArg(input_i.name(), mainGraph_ ? DNN_ARG_INPUT : DNN_ARG_TEMP); + Arg arg = netimpl->newArg(input_name, mainGraph_ ? DNN_ARG_INPUT : DNN_ARG_TEMP); if (!parseValueInfo(input_i, netimpl->args.at(arg.idx))) { raiseError(); + popRenames(local_renames); return Ptr(); } inputs.push_back(arg); @@ -812,9 +930,21 @@ Ptr ONNXImporter2::parseGraph(opencv_onnx::GraphProto* graph_proto, bool int n_outputs = graph_proto->output_size(); for (int i = 0; i < n_outputs; i++) { const opencv_onnx::ValueInfoProto& output_i = graph_proto->output(i); - Arg arg = netimpl->newArg(output_i.name(), mainGraph_ ? DNN_ARG_OUTPUT : DNN_ARG_TEMP); + std::string output_name = remap(output_i.name()); + Arg arg; + if (!output_name.empty() && net.haveArg(output_name)) { + arg = net.getArg(output_name); + if (mainGraph_) { + ArgData& adata = netimpl->args.at(arg.idx); + if (adata.kind == DNN_ARG_TEMP) + adata.kind = DNN_ARG_OUTPUT; + } + } else { + arg = netimpl->newArg(output_name, mainGraph_ ? DNN_ARG_OUTPUT : DNN_ARG_TEMP); + } if (!parseValueInfo(output_i, netimpl->args.at(arg.idx))) { raiseError(); + popRenames(local_renames); return Ptr(); } outputs.push_back(arg); @@ -825,7 +955,6 @@ Ptr ONNXImporter2::parseGraph(opencv_onnx::GraphProto* graph_proto, bool std::swap(saved_prog, curr_prog); - std::vector > prog; for (int i = 0; i < n_nodes && !have_errors; i++) { parseNode(graph_proto->node(i)); } @@ -837,6 +966,7 @@ Ptr ONNXImporter2::parseGraph(opencv_onnx::GraphProto* graph_proto, bool curr_graph_proto = saved_graph_proto; curr_graph = saved_graph; + popRenames(local_renames); return just_constructed; } @@ -907,22 +1037,18 @@ void ONNXImporter2::parseNode(const opencv_onnx::NodeProto& node_proto) int n_inputs = node_proto.input_size(); for (int i = 0; i < n_inputs; i++) { - const std::string& arg_name = node_proto.input(i); + std::string arg_name = remap(node_proto.input(i)); if (!net.haveArg(arg_name)) { CV_LOG_ERROR(NULL, "DNN/ONNX: unknown input '" << arg_name << "' of node '" << node_name << "'"); raiseError(); } Arg arg = net.getArg(arg_name); - /*ArgData adata = net.argData(arg); - printf("%s (%s), arg '%s'/'%s': adata.kind = %s, type=%s\n", node_name.c_str(), layer_type.c_str(), - arg_name.c_str(), adata.name.c_str(), - argKindToString(adata.kind).c_str(), typeToString(adata.type).c_str());*/ node_inputs.push_back(arg); } int n_outputs = node_proto.output_size(); for (int i = 0; i < n_outputs; i++) { - const std::string& arg_name = node_proto.output(i); + std::string arg_name = remap(node_proto.output(i)); Arg arg = net.getArg(arg_name); node_outputs.push_back(arg); } diff --git a/modules/dnn/test/test_layers.cpp b/modules/dnn/test/test_layers.cpp index 5be5b4eee9..f0322138e1 100644 --- a/modules/dnn/test/test_layers.cpp +++ b/modules/dnn/test/test_layers.cpp @@ -2864,6 +2864,51 @@ TEST(Layer_If, resize) } } +TEST(Layer_If, subgraph_name_scoping) +{ + auto engine_forced = static_cast( + cv::utils::getConfigurationParameterSizeT("OPENCV_FORCE_DNN_ENGINE", cv::dnn::ENGINE_AUTO)); + if (engine_forced == cv::dnn::ENGINE_CLASSIC) + { + applyTestTag(CV_TEST_TAG_DNN_SKIP_PARSER); + return; + } + + const std::string modelname = findDataFile("dnn/onnx/models/subgraph_name_scoping.onnx", true); + dnn::Net net = dnn::readNetFromONNX(modelname, ENGINE_NEW); + + int xshape[1] = {2}; + Mat x(1, xshape, CV_32F); + x.at(0) = 1.f; + x.at(1) = 2.f; + + for (int f = 0; f <= 1; f++) { + Mat cond(1, 1, CV_BoolC1, cv::Scalar(f)); + + net.setInput(cond, "cond"); + net.setInput(x.clone(), "x"); + + std::vector outs; + net.forward(outs, std::vector{"sum_outer", "branch_val"}); + ASSERT_EQ(outs.size(), 2u); + + // sum_outer = x + outer "shared" ([10, 20]). + const float* sumP = outs[0].ptr(); + EXPECT_FLOAT_EQ(sumP[0], 11.f); + EXPECT_FLOAT_EQ(sumP[1], 22.f); + + // branch_val is the body's locally-scoped "shared": [1, 2] or [100, 200]. + const float* brP = outs[1].ptr(); + if (f) { + EXPECT_FLOAT_EQ(brP[0], 1.f); + EXPECT_FLOAT_EQ(brP[1], 2.f); + } else { + EXPECT_FLOAT_EQ(brP[0], 100.f); + EXPECT_FLOAT_EQ(brP[1], 200.f); + } + } +} + TEST(Layer_Size, onnx_1d) { auto engine_forced = static_cast(