From dc1fe72647f4bd319331113b3971030e241e826a Mon Sep 17 00:00:00 2001
From: Olivier BICHLER <olivier.bichler@cea.fr>
Date: Tue, 18 Feb 2025 11:29:09 +0100
Subject: [PATCH 1/4] Implement proposal in #212
---
src/graph/GraphView.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/graph/GraphView.cpp b/src/graph/GraphView.cpp
index fab9be915..26c693152 100644
--- a/src/graph/GraphView.cpp
+++ b/src/graph/GraphView.cpp
@@ -1115,6 +1115,10 @@ void Aidge::GraphView::insertParent(NodePtr childNode,
}
bool Aidge::GraphView::replace(const std::set<Aidge::NodePtr>& oldNodes, const std::set<Aidge::NodePtr>& newNodes) {
+ AIDGE_ASSERT(!(oldNodes.size() > 1 && newNodes.size() > 1),
+ "GraphView::replace(): old and new sets cannot both have more than one node (here: {} and {} respectively). Use GraphView instead of set in this case.",
+ oldNodes.size(), newNodes.size());
+
// (1) create GraphViews from both sets of Nodes
auto oldG = std::make_shared<GraphView>("oldG");
oldG->add(oldNodes, false);
--
GitLab
From 37355ba95134322ead021618aa59667db208638f Mon Sep 17 00:00:00 2001
From: Olivier BICHLER <olivier.bichler@cea.fr>
Date: Tue, 18 Feb 2025 12:06:57 +0100
Subject: [PATCH 2/4] Changed wrong logic
---
src/graph/GraphView.cpp | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/graph/GraphView.cpp b/src/graph/GraphView.cpp
index 26c693152..b0d7336e2 100644
--- a/src/graph/GraphView.cpp
+++ b/src/graph/GraphView.cpp
@@ -1115,16 +1115,19 @@ void Aidge::GraphView::insertParent(NodePtr childNode,
}
bool Aidge::GraphView::replace(const std::set<Aidge::NodePtr>& oldNodes, const std::set<Aidge::NodePtr>& newNodes) {
- AIDGE_ASSERT(!(oldNodes.size() > 1 && newNodes.size() > 1),
- "GraphView::replace(): old and new sets cannot both have more than one node (here: {} and {} respectively). Use GraphView instead of set in this case.",
- oldNodes.size(), newNodes.size());
-
// (1) create GraphViews from both sets of Nodes
auto oldG = std::make_shared<GraphView>("oldG");
oldG->add(oldNodes, false);
auto newG = std::make_shared<GraphView>("newG");
newG->add(newNodes, false);
+ AIDGE_ASSERT(!(oldG->getOrderedInputs().size() > 1 || newG->getOrderedInputs().size() > 1),
+ "GraphView::replace(): old and new sets cannot have more than one input node (here: {} and {} respectively). Use GraphView instead of set in this case.",
+ oldG->getOrderedInputs().size(), newG->getOrderedInputs().size());
+ AIDGE_ASSERT(!(oldG->getOrderedOutputs().size() > 1 || newG->getOrderedOutputs().size() > 1),
+ "GraphView::replace(): old and new sets cannot have more than one output node (here: {} and {} respectively). Use GraphView instead of set in this case.",
+ oldG->getOrderedOutputs().size() , newG->getOrderedOutputs().size());
+
return GraphView::replace(oldG, newG);
}
--
GitLab
From d456e979b37695e0bd57ddd11747e91b82497871 Mon Sep 17 00:00:00 2001
From: Olivier BICHLER <olivier.bichler@cea.fr>
Date: Tue, 18 Feb 2025 16:03:30 +0100
Subject: [PATCH 3/4] More work was necessary
---
src/graph/GraphView.cpp | 33 +++++++++++++++++++++++------
unit_tests/graph/Test_GraphView.cpp | 7 ++++--
2 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/src/graph/GraphView.cpp b/src/graph/GraphView.cpp
index b0d7336e2..1f1772c59 100644
--- a/src/graph/GraphView.cpp
+++ b/src/graph/GraphView.cpp
@@ -1114,6 +1114,26 @@ void Aidge::GraphView::insertParent(NodePtr childNode,
add(newParentNode);
}
+/**
+ * old \ new | 1 node, 1 input | >1 node, 1 input | 1 node, >1 inputs | >1 node, >1 inputs
+ * ----------------------------------------------------------------------------------------------------
+ * 1 node, 1 input | trivial | trivial | broadcast | broadcast
+ * >1 node, 1 input | trivial | trivial | broadcast | broadcast
+ * 1 node, >1 inputs | (take first) | (take first) | same order | X
+ * >1 node, >1 inputs | X | X | X | X
+ *
+ * old \ new | 1 node, 1 output | >1 node, 1 output | 1 node, >1 outputs | >1 node, >1 outputs
+ * -----------------------------------------------------------------------------------------------------
+ * 1 node, 1 output | trivial | trivial | take first | X
+ * >1 node, 1 output | trivial | trivial | take first | X
+ * 1 node, >1 outputs | (take first) | (take first) | same order | X
+ * >1 node, >1 outputs | X | X | X | X
+ *
+ * Only the X cases cannot possibly be resolved deterministically with sets of node.
+ * These cases are therefore forbidden for the set-based replace() interface.
+ * The remaining cases are handled by the GraphView-based replace() interface.
+ * If they are not supported, the function returns false.
+ */
bool Aidge::GraphView::replace(const std::set<Aidge::NodePtr>& oldNodes, const std::set<Aidge::NodePtr>& newNodes) {
// (1) create GraphViews from both sets of Nodes
auto oldG = std::make_shared<GraphView>("oldG");
@@ -1121,12 +1141,13 @@ bool Aidge::GraphView::replace(const std::set<Aidge::NodePtr>& oldNodes, const s
auto newG = std::make_shared<GraphView>("newG");
newG->add(newNodes, false);
- AIDGE_ASSERT(!(oldG->getOrderedInputs().size() > 1 || newG->getOrderedInputs().size() > 1),
- "GraphView::replace(): old and new sets cannot have more than one input node (here: {} and {} respectively). Use GraphView instead of set in this case.",
- oldG->getOrderedInputs().size(), newG->getOrderedInputs().size());
- AIDGE_ASSERT(!(oldG->getOrderedOutputs().size() > 1 || newG->getOrderedOutputs().size() > 1),
- "GraphView::replace(): old and new sets cannot have more than one output node (here: {} and {} respectively). Use GraphView instead of set in this case.",
- oldG->getOrderedOutputs().size() , newG->getOrderedOutputs().size());
+ AIDGE_ASSERT(!((oldNodes.size() > 1 && oldG->getOrderedInputs().size() > 1) || (newNodes.size() > 1 && newG->getOrderedInputs().size() > 1 && oldG->getOrderedInputs().size() > 1)),
+ "GraphView::replace(): don't know how to match {} input(s) from {} node(s) (old set) to {} input(s) from {} node(s) (new set). Use GraphView instead of set in this case.",
+ oldG->getOrderedInputs().size(), oldNodes.size(), newG->getOrderedInputs().size(), newNodes.size());
+
+ AIDGE_ASSERT(!((oldNodes.size() > 1 && oldG->getOrderedOutputs().size() > 1) || (newNodes.size() > 1 && newG->getOrderedOutputs().size() > 1)),
+ "GraphView::replace(): don't know how to match {} output(s) from {} node(s) (old set) to {} output(s) from {} node(s) (new set). Use GraphView instead of set in this case.",
+ oldG->getOrderedOutputs().size(), oldNodes.size(), newG->getOrderedOutputs().size(), newNodes.size());
return GraphView::replace(oldG, newG);
}
diff --git a/unit_tests/graph/Test_GraphView.cpp b/unit_tests/graph/Test_GraphView.cpp
index 5bd435e28..d2f41269e 100644
--- a/unit_tests/graph/Test_GraphView.cpp
+++ b/unit_tests/graph/Test_GraphView.cpp
@@ -659,9 +659,12 @@ TEST_CASE("[core/graph] GraphView(replace)", "[GraphView][replace]") {
REQUIRE(g->getNodes() == std::set<std::shared_ptr<Node>>({other1, conv1, conv2, conv3, conv4, concat, other2}));
- GraphView::replace({conv1, conv2, conv3, conv4, concat}, {myConv});
+ // This doesn't make sense in the general case: replace() cannot possibly
+ // know how to match the inputs of the 4 conv with the single input of myCond
+ // The implicit assumption here is that they are the same!
+ //GraphView::replace({conv1, conv2, conv3, conv4, concat}, {myConv});
- REQUIRE(g->getNodes() == std::set<std::shared_ptr<Node>>({other1, myConv, other2}));
+ //REQUIRE(g->getNodes() == std::set<std::shared_ptr<Node>>({other1, myConv, other2}));
}
SECTION("replace same input category 1") {
--
GitLab
From 4057db9eb344f694be2963d31187207561786804 Mon Sep 17 00:00:00 2001
From: Olivier BICHLER <olivier.bichler@cea.fr>
Date: Tue, 18 Feb 2025 16:10:57 +0100
Subject: [PATCH 4/4] Fixed table syntax
---
src/graph/GraphView.cpp | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/graph/GraphView.cpp b/src/graph/GraphView.cpp
index 1f1772c59..315844858 100644
--- a/src/graph/GraphView.cpp
+++ b/src/graph/GraphView.cpp
@@ -1115,23 +1115,25 @@ void Aidge::GraphView::insertParent(NodePtr childNode,
}
/**
- * old \ new | 1 node, 1 input | >1 node, 1 input | 1 node, >1 inputs | >1 node, >1 inputs
- * ----------------------------------------------------------------------------------------------------
- * 1 node, 1 input | trivial | trivial | broadcast | broadcast
- * >1 node, 1 input | trivial | trivial | broadcast | broadcast
- * 1 node, >1 inputs | (take first) | (take first) | same order | X
- * >1 node, >1 inputs | X | X | X | X
+ * Inputs conditions:
+ * | old \ new | 1 node, 1 input | >1 node, 1 input | 1 node, >1 inputs | >1 node, >1 inputs |
+ * | ------------------- | ---------------- | ----------------- | ------------------ | ------------------ |
+ * | 1 node, 1 input | trivial | trivial | broadcast | broadcast |
+ * | >1 node, 1 input | trivial | trivial | broadcast | broadcast |
+ * | 1 node, >1 inputs | (take first) | (take first) | same order | X |
+ * | >1 node, >1 inputs | X | X | X | X |
*
- * old \ new | 1 node, 1 output | >1 node, 1 output | 1 node, >1 outputs | >1 node, >1 outputs
- * -----------------------------------------------------------------------------------------------------
- * 1 node, 1 output | trivial | trivial | take first | X
- * >1 node, 1 output | trivial | trivial | take first | X
- * 1 node, >1 outputs | (take first) | (take first) | same order | X
- * >1 node, >1 outputs | X | X | X | X
+ * Outputs conditions:
+ * | old \ new | 1 node, 1 output | >1 node, 1 output | 1 node, >1 outputs | >1 node, >1 outputs |
+ * | ------------------- | ---------------- | ----------------- | ------------------ | ------------------- |
+ * | 1 node, 1 output | trivial | trivial | take first | X |
+ * | >1 node, 1 output | trivial | trivial | take first | X |
+ * | 1 node, >1 outputs | (take first) | (take first) | same order | X |
+ * | >1 node, >1 outputs | X | X | X | X |
*
* Only the X cases cannot possibly be resolved deterministically with sets of node.
- * These cases are therefore forbidden for the set-based replace() interface.
- * The remaining cases are handled by the GraphView-based replace() interface.
+ * These cases are therefore forbidden for the set-based `replace()` interface.
+ * The remaining cases are handled by the GraphView-based `replace()` interface.
* If they are not supported, the function returns false.
*/
bool Aidge::GraphView::replace(const std::set<Aidge::NodePtr>& oldNodes, const std::set<Aidge::NodePtr>& newNodes) {
--
GitLab