From f050ac318301f14491a6c31d973d14c596ae2a74 Mon Sep 17 00:00:00 2001
From: Olivier BICHLER <olivier.bichler@cea.fr>
Date: Tue, 3 Sep 2024 09:28:47 +0200
Subject: [PATCH] Improved binding

---
 include/aidge/backend/OperatorImpl.hpp        |  8 ++--
 include/aidge/operator/ReduceMean.hpp         |  2 +
 .../backend/pybind_OperatorImpl.cpp           | 12 +++---
 python_binding/data/pybind_Data.cpp           | 43 +++++++++++--------
 python_binding/pybind_core.cpp                |  4 +-
 src/backend/OperatorImpl.cpp                  |  6 +--
 src/operator/ReduceMean.cpp                   |  2 +
 7 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/include/aidge/backend/OperatorImpl.hpp b/include/aidge/backend/OperatorImpl.hpp
index 9a184acfd..9c76f49b8 100644
--- a/include/aidge/backend/OperatorImpl.hpp
+++ b/include/aidge/backend/OperatorImpl.hpp
@@ -32,7 +32,7 @@ class Operator;
  */
 struct ImplSpec {
     struct IOSpec {
-        IOSpec(DataType type_, DataFormat format_ = DataFormat::Any, std::vector<std::pair<int, int>> dims_ = {}):
+        IOSpec(DataType type_, DataFormat format_ = DataFormat::Any, const std::vector<std::pair<int, int>>& dims_ = {}):
             type(type_),
             format(format_),
             dims(dims_)
@@ -43,9 +43,9 @@ struct ImplSpec {
         std::vector<std::pair<int, int>> dims;
     };
 
-    ImplSpec(DynamicAttributes attrs_ = DynamicAttributes());
-    ImplSpec(IOSpec io, DynamicAttributes attrs_ = DynamicAttributes());
-    ImplSpec(IOSpec i, IOSpec o, DynamicAttributes attrs_ = DynamicAttributes());
+    ImplSpec(const DynamicAttributes& attrs_ = DynamicAttributes());
+    ImplSpec(const IOSpec& io, const DynamicAttributes& attrs_ = DynamicAttributes());
+    ImplSpec(const IOSpec& i, const IOSpec& o, const DynamicAttributes& attrs_ = DynamicAttributes());
     ImplSpec(const Aidge::ImplSpec&);
     ~ImplSpec() noexcept;
 
diff --git a/include/aidge/operator/ReduceMean.hpp b/include/aidge/operator/ReduceMean.hpp
index ec7a0d4cd..f892d5a38 100644
--- a/include/aidge/operator/ReduceMean.hpp
+++ b/include/aidge/operator/ReduceMean.hpp
@@ -74,6 +74,8 @@ public:
     static const std::vector<std::string> getOutputsName() {
         return {"data_output"};
     }
+
+    virtual ~ReduceMean_Op() noexcept;
 };
 
 /**
diff --git a/python_binding/backend/pybind_OperatorImpl.cpp b/python_binding/backend/pybind_OperatorImpl.cpp
index bc2321ab1..a129af8a2 100644
--- a/python_binding/backend/pybind_OperatorImpl.cpp
+++ b/python_binding/backend/pybind_OperatorImpl.cpp
@@ -71,14 +71,14 @@ public:
 };
 
 void init_OperatorImpl(py::module& m){
-    py::class_<ImplSpec>(m, "ImplSpec")
-    .def(py::init<DynamicAttributes>())
-    .def(py::init<ImplSpec::IOSpec, DynamicAttributes>())
-    .def(py::init<ImplSpec::IOSpec, ImplSpec::IOSpec, DynamicAttributes>())
+    py::class_<ImplSpec::IOSpec>(m, "IOSpec")
+    .def(py::init<DataType, DataFormat, const std::vector<std::pair<int, int>>&>(), py::arg("type"), py::arg("format") = DataFormat::Any, py::arg("dims") = std::vector<std::pair<int, int>>{})
     ;
 
-    py::class_<ImplSpec::IOSpec>(m, "IOSpec")
-    .def(py::init<DataType, DataFormat, std::vector<std::pair<int, int>>>())
+    py::class_<ImplSpec>(m, "ImplSpec")
+    .def(py::init<const DynamicAttributes&>(), py::arg("attr") = DynamicAttributes())
+    .def(py::init<const ImplSpec::IOSpec&, const DynamicAttributes&>(), py::arg("io"), py::arg("attr") = DynamicAttributes())
+    .def(py::init<const ImplSpec::IOSpec&, const ImplSpec::IOSpec&, const DynamicAttributes&>(), py::arg("i"), py::arg("o"), py::arg("attr") = DynamicAttributes())
     ;
 
     py::class_<OperatorImpl, std::shared_ptr<OperatorImpl>, pyOperatorImpl>(m, "OperatorImpl", py::dynamic_attr())
diff --git a/python_binding/data/pybind_Data.cpp b/python_binding/data/pybind_Data.cpp
index 1d4eae077..e91f345d7 100644
--- a/python_binding/data/pybind_Data.cpp
+++ b/python_binding/data/pybind_Data.cpp
@@ -16,46 +16,51 @@
 namespace py = pybind11;
 namespace Aidge {
 
-void init_Data(py::module& m){
-    // Define enumeration names for python as lowercase dtype name
-    // This defined enum names compatible with basic numpy dtype
+template <class T>
+void bindEnum(py::module& m, const std::string& name) {
+    // Define enumeration names for python as lowercase type name
+    // This defined enum names compatible with basic numpy type
     // name such as: float32, flot64, [u]int32, [u]int64, ...
-    auto python_enum_name = [](const DataType& dtype) {
+    auto python_enum_name = [](const T& type) {
         auto str_lower = [](std::string& str) {
             std::transform(str.begin(), str.end(), str.begin(),
                            [](unsigned char c){
                                return std::tolower(c);
                            });
         };
-        auto dtype_name = std::string(Aidge::format_as(dtype));
-        str_lower(dtype_name);
-        return dtype_name;
+        auto type_name = std::string(Aidge::format_as(type));
+        str_lower(type_name);
+        return type_name;
     };
-    // Auto generate enumeration names from lowercase dtype strings
+    // Auto generate enumeration names from lowercase type strings
     std::vector<std::string> enum_names;
-    for (auto dtype_str : EnumStrings<Aidge::DataType>::data) {
-        auto dtype = static_cast<DataType>(enum_names.size());
-        auto enum_name = python_enum_name(dtype);
+    for (auto type_str : EnumStrings<T>::data) {
+        auto type = static_cast<T>(enum_names.size());
+        auto enum_name = python_enum_name(type);
         enum_names.push_back(enum_name);
     }
 
-    // Define python side enumeration aidge_core.dtype
-    auto e_dtype = py::enum_<DataType>(m, "dtype");
+    // Define python side enumeration aidge_core.type
+    auto e_type = py::enum_<T>(m, name.c_str());
 
     // Add enum value for each enum name
     for (std::size_t idx = 0; idx < enum_names.size(); idx++) {
-        e_dtype.value(enum_names[idx].c_str(), static_cast<DataType>(idx));
+        e_type.value(enum_names[idx].c_str(), static_cast<T>(idx));
     }
 
     // Define str() to return the bare enum name value, it allows
-    // to compare directly for instance str(tensor.dtype())
-    // with str(nparray.dtype)
-    e_dtype.def("__str__", [enum_names](const DataType& dtype) {
-        return enum_names[static_cast<int>(dtype)];
+    // to compare directly for instance str(tensor.type())
+    // with str(nparray.type)
+    e_type.def("__str__", [enum_names](const T& type) {
+        return enum_names[static_cast<int>(type)];
     }, py::prepend());;
+}
 
-    py::class_<Data, std::shared_ptr<Data>>(m,"Data");
+void init_Data(py::module& m){
+    bindEnum<DataType>(m, "dtype");
+    bindEnum<DataFormat>(m, "dformat");
 
+    py::class_<Data, std::shared_ptr<Data>>(m,"Data");
 
 }
 }
diff --git a/python_binding/pybind_core.cpp b/python_binding/pybind_core.cpp
index 0ed7f4e4c..9cf240d14 100644
--- a/python_binding/pybind_core.cpp
+++ b/python_binding/pybind_core.cpp
@@ -21,8 +21,8 @@ void init_Data(py::module&);
 void init_Database(py::module&);
 void init_DataProvider(py::module&);
 void init_Tensor(py::module&);
-void init_OperatorImpl(py::module&);
 void init_Attributes(py::module&);
+void init_OperatorImpl(py::module&);
 void init_Log(py::module&);
 void init_Operator(py::module&);
 void init_OperatorTensor(py::module&);
@@ -89,6 +89,7 @@ void init_Aidge(py::module& m) {
     init_Database(m);
     init_DataProvider(m);
     init_Tensor(m);
+    init_Attributes(m);
 
     init_Node(m);
     init_GraphView(m);
@@ -96,7 +97,6 @@ void init_Aidge(py::module& m) {
     init_Connector(m);
 
     init_OperatorImpl(m);
-    init_Attributes(m);
     init_Log(m);
     init_Operator(m);
     init_OperatorTensor(m);
diff --git a/src/backend/OperatorImpl.cpp b/src/backend/OperatorImpl.cpp
index 4112eee07..5b53f9bef 100644
--- a/src/backend/OperatorImpl.cpp
+++ b/src/backend/OperatorImpl.cpp
@@ -22,11 +22,11 @@
 #include "aidge/data/Tensor.hpp"
 #include "aidge/utils/ErrorHandling.hpp"
 
-Aidge::ImplSpec::ImplSpec(DynamicAttributes attrs_):
+Aidge::ImplSpec::ImplSpec(const DynamicAttributes& attrs_):
     attrs(attrs_) {}
-Aidge::ImplSpec::ImplSpec(IOSpec io, DynamicAttributes attrs_):
+Aidge::ImplSpec::ImplSpec(const IOSpec& io, const DynamicAttributes& attrs_):
     inputs(1, io), outputs(1, io), attrs(attrs_) {}
-Aidge::ImplSpec::ImplSpec(IOSpec i, IOSpec o, DynamicAttributes attrs_):
+Aidge::ImplSpec::ImplSpec(const IOSpec& i, const IOSpec& o, const DynamicAttributes& attrs_):
     inputs(1, i), outputs(1, o), attrs(attrs_) {}
 Aidge::ImplSpec::ImplSpec(const Aidge::ImplSpec&) = default;
 Aidge::ImplSpec::~ImplSpec() noexcept = default;
diff --git a/src/operator/ReduceMean.cpp b/src/operator/ReduceMean.cpp
index d80525adc..73af42ac8 100644
--- a/src/operator/ReduceMean.cpp
+++ b/src/operator/ReduceMean.cpp
@@ -80,6 +80,8 @@ void Aidge::ReduceMean_Op::setBackend(const std::string& name, Aidge::DeviceIdx_
     mOutputs[0]->setBackend(name, device);
 }
 
+Aidge::ReduceMean_Op::~ReduceMean_Op() noexcept = default;
+
 ////////////////////////////////////////////
 
 std::shared_ptr<Aidge::Node> Aidge::ReduceMean(const std::vector<std::int32_t> &axes,
-- 
GitLab