From 2d586688f94a81ced3adf2040a2fe8016bcc3cf4 Mon Sep 17 00:00:00 2001
From: Olivier BICHLER <olivier.bichler@cea.fr>
Date: Mon, 25 Sep 2023 18:31:01 +0200
Subject: [PATCH] Updated after review
---
include/aidge/backend/OperatorImpl.hpp | 6 ++++++
include/aidge/graph/GraphView.hpp | 8 ++++----
include/aidge/graph/Node.hpp | 12 ++++++------
include/aidge/operator/Add.hpp | 5 ++---
include/aidge/operator/AvgPooling.hpp | 5 ++---
include/aidge/operator/BatchNorm.hpp | 5 ++---
include/aidge/operator/Conv.hpp | 5 ++---
include/aidge/operator/ConvDepthWise.hpp | 5 ++---
include/aidge/operator/FC.hpp | 5 ++---
include/aidge/operator/GenericOperator.hpp | 6 +++---
include/aidge/operator/LeakyReLU.hpp | 5 ++---
include/aidge/operator/Matmul.hpp | 5 ++---
include/aidge/operator/MaxPooling.hpp | 5 ++---
include/aidge/operator/MetaOperator.hpp | 5 ++---
include/aidge/operator/Operator.hpp | 5 ++---
include/aidge/operator/Producer.hpp | 5 ++---
include/aidge/operator/ReLU.hpp | 5 ++---
include/aidge/operator/Softmax.hpp | 5 ++---
18 files changed, 47 insertions(+), 55 deletions(-)
diff --git a/include/aidge/backend/OperatorImpl.hpp b/include/aidge/backend/OperatorImpl.hpp
index d10270b62..cc77ab053 100644
--- a/include/aidge/backend/OperatorImpl.hpp
+++ b/include/aidge/backend/OperatorImpl.hpp
@@ -14,11 +14,17 @@
#include <cstddef>
#include <vector>
+#include <memory>
#include "aidge/utils/Types.h"
namespace Aidge {
class OperatorImpl {
public:
+ /**
+ * @brief Clone the implementation using its copy-constructor.
+ */
+ virtual std::unique_ptr<OperatorImpl> clone() const = 0;
+
virtual void forward(){};
virtual void backward(){};
diff --git a/include/aidge/graph/GraphView.hpp b/include/aidge/graph/GraphView.hpp
index deabfdec4..6a29c960e 100644
--- a/include/aidge/graph/GraphView.hpp
+++ b/include/aidge/graph/GraphView.hpp
@@ -337,7 +337,7 @@ public:
void updateOutputNodes();
/**
- * @brief Make a clone of the graph with shared operators. It a new graph, with new, cloned nodes, but the new nodes refer to the same operators as the origin ones.
+ * @brief Clone the GraphView with shared Operators. It is a new GraphView, with cloned Nodes, but the new Nodes refer to the same Operators as the original ones.
* @return std::shared_ptr<GraphView>
*/
inline std::shared_ptr<GraphView> cloneSharedOperators() const {
@@ -345,7 +345,7 @@ public:
}
/**
- * @brief Make a clone of the graph with shared producers. All the other operators are copied.
+ * @brief Clone the GraphView with shared Producers. All the other Operators are copied.
* @return std::shared_ptr<GraphView>
*/
inline std::shared_ptr<GraphView> cloneSharedProducers() const {
@@ -353,7 +353,7 @@ public:
}
/**
- * @brief Make a clone of the graph. Everything is cloned: nodes and operators.
+ * @brief Clone the GraphView. Everything is cloned: Nodes and Operators.
* @return std::shared_ptr<GraphView>
*/
inline std::shared_ptr<GraphView> clone() const {
@@ -361,7 +361,7 @@ public:
}
/**
- * @brief This function clones the graph using a callback function for the node cloning, allowing to specify how each node should be cloned, or replaced by an other node type, or removed (i.e. replaced by identity). When a node is removed, the clone() method automatically finds the next valid parent in line, going backward in the graph and connects it if that makes sense without ambiguity (effectively treating the removed node as an identity operation).
+ * @brief Clone the current GraphView using a callback function for the Node cloning, allowing to specify how each Node should be cloned or replaced by another Node type, or removed (i.e. replaced by identity). When a Node is removed, the clone() method automatically finds the next valid parent in line, going backward in the graph and connects it if that makes sense without ambiguity (effectively treating the removed Node as an identity operation).
* @param cloneNode Callback function to clone a node
* @return std::shared_ptr<GraphView>
*/
diff --git a/include/aidge/graph/Node.hpp b/include/aidge/graph/Node.hpp
index c3bc4b112..dbe017fc7 100644
--- a/include/aidge/graph/Node.hpp
+++ b/include/aidge/graph/Node.hpp
@@ -355,25 +355,25 @@ public:
///////////////////////////////////////////////////////
/**
- * @brief Clone the node keeping the same operator object instance. The new node has no connection.
+ * @brief Clone the current Node. The Operator attribute of the new Node is not copied but shared with the current Node. The new node has no connection.
* @return NodePtr
*/
NodePtr cloneSharedOperators() const;
/**
- * @brief Clone the node keeping the same operator object instance only for producers. Any other operator object instance is cloned as wel. The new node has no connection.
+ * @brief Clone the Node. Every attribute is copied, even Operator pointer except for Producers for which it is shared. The new Node has no connection.
* @return NodePtr
*/
NodePtr cloneSharedProducers() const;
/**
- * @brief Clone the node and its operator. The new node has no connection.
+ * @brief Clone the Node and its Operator. The new Node has no connection.
* @return NodePtr
*/
NodePtr clone() const;
/**
- * @brief Callback function to clone the node keeping the same operator object instance. The new node has no connection.
+ * @brief Callback function to clone the Node keeping the same Operator object instance. The new Node has no connection.
* @param node Node to clone.
* @return NodePtr
*/
@@ -382,7 +382,7 @@ public:
}
/**
- * @brief Callback function to clone the node keeping the same operator object instance only for producers. Any other operator object instance is cloned as wel. The new node has no connection.
+ * @brief Callback function to clone the Node. Every attribute is copied, even Operator pointer except for Producers for which it is shared. The new Node has no connection.
* @param node Node to clone.
* @return NodePtr
*/
@@ -391,7 +391,7 @@ public:
}
/**
- * @brief Callback function to clone the node and its operator. The new node has no connection.
+ * @brief Callback function to clone the Node and its Operator. The new Node has no connection.
* @param node Node to clone.
* @return NodePtr
*/
diff --git a/include/aidge/operator/Add.hpp b/include/aidge/operator/Add.hpp
index 3502e76ae..8991a6f5f 100644
--- a/include/aidge/operator/Add.hpp
+++ b/include/aidge/operator/Add.hpp
@@ -66,10 +66,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::Add_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new Add_Op(*static_cast<const Add_Op*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<Add_Op>(*this);
}
// Data operator[](const char* inputName) override final {
diff --git a/include/aidge/operator/AvgPooling.hpp b/include/aidge/operator/AvgPooling.hpp
index 6174fa497..4f6c2e6ce 100644
--- a/include/aidge/operator/AvgPooling.hpp
+++ b/include/aidge/operator/AvgPooling.hpp
@@ -78,10 +78,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::AvgPooling_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new AvgPooling_Op<DIM>(*static_cast<const AvgPooling_Op<DIM>*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<AvgPooling_Op<DIM>>(*this);
}
constexpr void associateInput(const IOIndex_t inputIdx, std::shared_ptr<Data> data) override final {
diff --git a/include/aidge/operator/BatchNorm.hpp b/include/aidge/operator/BatchNorm.hpp
index 51a2c7832..cb22eda33 100644
--- a/include/aidge/operator/BatchNorm.hpp
+++ b/include/aidge/operator/BatchNorm.hpp
@@ -71,10 +71,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::BatchNorm_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new BatchNorm_Op<DIM>(*static_cast<const BatchNorm_Op<DIM>*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<BatchNorm_Op<DIM>>(*this);
}
// Data operator[](const char* inputName) override final {
diff --git a/include/aidge/operator/Conv.hpp b/include/aidge/operator/Conv.hpp
index 63a5d59a0..3015069ea 100644
--- a/include/aidge/operator/Conv.hpp
+++ b/include/aidge/operator/Conv.hpp
@@ -81,10 +81,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::Conv_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new Conv_Op<DIM>(*static_cast<const Conv_Op<DIM>*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<Conv_Op<DIM>>(*this);
}
// Data operator[](const char* inputName) override final {
diff --git a/include/aidge/operator/ConvDepthWise.hpp b/include/aidge/operator/ConvDepthWise.hpp
index 8485f09ba..3d0e5c931 100644
--- a/include/aidge/operator/ConvDepthWise.hpp
+++ b/include/aidge/operator/ConvDepthWise.hpp
@@ -86,10 +86,9 @@ class ConvDepthWise_Op : public Operator,
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::ConvDepthWise_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new ConvDepthWise_Op<DIM>(*static_cast<const ConvDepthWise_Op<DIM>*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<ConvDepthWise_Op<DIM>>(*this);
}
constexpr void associateInput(const IOIndex_t inputIdx, std::shared_ptr<Data> data) override final {
diff --git a/include/aidge/operator/FC.hpp b/include/aidge/operator/FC.hpp
index 8a9592369..c17344694 100644
--- a/include/aidge/operator/FC.hpp
+++ b/include/aidge/operator/FC.hpp
@@ -72,10 +72,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::FC_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new FC_Op(*static_cast<const FC_Op*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<FC_Op>(*this);
}
void associateInput(const IOIndex_t inputIdx, std::shared_ptr<Data> data) override final {
diff --git a/include/aidge/operator/GenericOperator.hpp b/include/aidge/operator/GenericOperator.hpp
index ba2168705..9439ebd33 100644
--- a/include/aidge/operator/GenericOperator.hpp
+++ b/include/aidge/operator/GenericOperator.hpp
@@ -16,6 +16,7 @@
#include <vector>
#include <string>
#include <cassert>
+#include <cstring>
#include "aidge/graph/Node.hpp"
#include "aidge/operator/Operator.hpp"
@@ -73,10 +74,9 @@ class GenericOperator_Op
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::GenericOperator_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new GenericOperator_Op(*static_cast<const GenericOperator_Op*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<GenericOperator_Op>(*this);
}
/**
diff --git a/include/aidge/operator/LeakyReLU.hpp b/include/aidge/operator/LeakyReLU.hpp
index e57fdff70..2553b46d8 100644
--- a/include/aidge/operator/LeakyReLU.hpp
+++ b/include/aidge/operator/LeakyReLU.hpp
@@ -69,10 +69,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::LeakyReLU_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new LeakyReLU_Op(*static_cast<const LeakyReLU_Op*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<LeakyReLU_Op>(*this);
}
void associateInput(const IOIndex_t inputIdx, std::shared_ptr<Data> data) override final {
diff --git a/include/aidge/operator/Matmul.hpp b/include/aidge/operator/Matmul.hpp
index 5536b70ef..fc0032951 100644
--- a/include/aidge/operator/Matmul.hpp
+++ b/include/aidge/operator/Matmul.hpp
@@ -70,10 +70,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::Matmul_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new Matmul_Op(*static_cast<const Matmul_Op*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<Matmul_Op>(*this);
}
void associateInput(const IOIndex_t inputIdx, std::shared_ptr<Data> data) override final {
diff --git a/include/aidge/operator/MaxPooling.hpp b/include/aidge/operator/MaxPooling.hpp
index 49acccc49..5eff15f72 100644
--- a/include/aidge/operator/MaxPooling.hpp
+++ b/include/aidge/operator/MaxPooling.hpp
@@ -79,10 +79,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::MaxPooling_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new MaxPooling_Op<DIM>(*static_cast<const MaxPooling_Op<DIM>*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<MaxPooling_Op<DIM>>(*this);
}
constexpr void associateInput(const IOIndex_t inputIdx, std::shared_ptr<Data> data) override final {
diff --git a/include/aidge/operator/MetaOperator.hpp b/include/aidge/operator/MetaOperator.hpp
index 20dfb2e12..9e12b1598 100644
--- a/include/aidge/operator/MetaOperator.hpp
+++ b/include/aidge/operator/MetaOperator.hpp
@@ -35,10 +35,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::Matmul_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new MetaOperator(*static_cast<const MetaOperator*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<MetaOperator>(*this);
}
~MetaOperator() = default;
diff --git a/include/aidge/operator/Operator.hpp b/include/aidge/operator/Operator.hpp
index 0289b92b8..925249374 100644
--- a/include/aidge/operator/Operator.hpp
+++ b/include/aidge/operator/Operator.hpp
@@ -33,15 +33,14 @@ private:
public:
Operator() = delete;
Operator(const char* type) : mType(type) {}
- virtual Operator* clone() const = 0;
+ virtual std::shared_ptr<Operator> clone() const = 0;
virtual ~Operator();
Operator(const Operator& op):
std::enable_shared_from_this<Operator>()
{
mType = op.mType;
- // mImpl is not set right now.
- // TODO: clone the impl as well?
+ mImpl = op.mImpl->clone();
}
public:
diff --git a/include/aidge/operator/Producer.hpp b/include/aidge/operator/Producer.hpp
index 907dff559..937105fbb 100644
--- a/include/aidge/operator/Producer.hpp
+++ b/include/aidge/operator/Producer.hpp
@@ -65,10 +65,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::Producer_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new Producer_Op(*static_cast<const Producer_Op*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<Producer_Op>(*this);
}
void associateInput(const IOIndex_t /*inputIdx*/, std::shared_ptr<Data> /*data*/) override final {
diff --git a/include/aidge/operator/ReLU.hpp b/include/aidge/operator/ReLU.hpp
index 8178ad9fb..2f9751a82 100644
--- a/include/aidge/operator/ReLU.hpp
+++ b/include/aidge/operator/ReLU.hpp
@@ -57,10 +57,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::ReLU_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new ReLU_Op(*static_cast<const ReLU_Op*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<ReLU_Op>(*this);
}
void associateInput(const IOIndex_t inputIdx, std::shared_ptr<Data> data) override final {
diff --git a/include/aidge/operator/Softmax.hpp b/include/aidge/operator/Softmax.hpp
index 1aa7da902..193fd811b 100644
--- a/include/aidge/operator/Softmax.hpp
+++ b/include/aidge/operator/Softmax.hpp
@@ -57,10 +57,9 @@ public:
/**
* @brief Clone the operator using its copy-constructor.
* @see Operator::Softmax_Op
- * @param op Operator to copy.
*/
- Operator* clone() const override {
- return new Softmax_Op(*static_cast<const Softmax_Op*>(this));
+ std::shared_ptr<Operator> clone() const override {
+ return std::make_shared<Softmax_Op>(*this);
}
void associateInput(const IOIndex_t inputIdx, std::shared_ptr<Data> data) override final {
--
GitLab