From 47aa7d1310e45f40850927d0d788868286a1a4fa Mon Sep 17 00:00:00 2001
From: Olivier BICHLER <olivier.bichler@cea.fr>
Date: Tue, 13 Feb 2024 17:10:01 +0100
Subject: [PATCH] Improved error messages

---
 include/aidge/operator/AvgPooling.hpp    |  2 +-
 include/aidge/operator/Concat.hpp        |  2 +-
 include/aidge/operator/Conv.hpp          |  2 +-
 include/aidge/operator/ConvDepthWise.hpp |  2 +-
 include/aidge/operator/FC.hpp            |  2 +-
 include/aidge/operator/MatMul.hpp        |  2 +-
 include/aidge/operator/MaxPooling.hpp    |  2 +-
 include/aidge/operator/MetaOperator.hpp  |  1 +
 include/aidge/operator/Pad.hpp           |  2 +-
 include/aidge/utils/ErrorHandling.hpp    |  1 +
 src/graph/GraphView.cpp                  |  9 ++++++++-
 src/operator/Memorize.cpp                |  6 ++++--
 src/operator/Mul.cpp                     |  2 +-
 src/operator/Operator.cpp                | 10 +++++-----
 src/operator/OperatorTensor.cpp          |  6 ++++--
 src/operator/Slice.cpp                   |  2 +-
 16 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/include/aidge/operator/AvgPooling.hpp b/include/aidge/operator/AvgPooling.hpp
index d7b77396f..5066cb78f 100644
--- a/include/aidge/operator/AvgPooling.hpp
+++ b/include/aidge/operator/AvgPooling.hpp
@@ -75,7 +75,7 @@ public:
     void computeOutputDims() override final {
         // check inputs have been associated
         if (!getInput(0)) {
-            AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+            AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #0 should be associated with a Tensor", type());
         }
         if (!(getInput(0)->empty())) {
             std::array<DimSize_t, DIM + 2> outputDims;
diff --git a/include/aidge/operator/Concat.hpp b/include/aidge/operator/Concat.hpp
index 08df7822a..62a954010 100644
--- a/include/aidge/operator/Concat.hpp
+++ b/include/aidge/operator/Concat.hpp
@@ -84,7 +84,7 @@ public:
         const auto firstInputNbDims = getInput(0) -> nbDims();
         for (IOIndex_t i = 1; i < nbInputs(); ++i) {
             if (!getInput(i)) {
-                AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+                AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #{} should be associated with a Tensor", type(), i);
             }
 
             if (getInput(i)->nbDims() == firstInputNbDims) {
diff --git a/include/aidge/operator/Conv.hpp b/include/aidge/operator/Conv.hpp
index 4483c2df2..8290fb3d0 100644
--- a/include/aidge/operator/Conv.hpp
+++ b/include/aidge/operator/Conv.hpp
@@ -94,7 +94,7 @@ public:
         bool associated = true;
         for (IOIndex_t i = 0; i < 3; ++i) {
             if (!getInput(i)) {
-                AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+                AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #{} should be associated with a Tensor", type(), i);
             }
             associated &= !(getInput(i)->empty());
         }
diff --git a/include/aidge/operator/ConvDepthWise.hpp b/include/aidge/operator/ConvDepthWise.hpp
index 1a4c867b3..a3b537ba6 100644
--- a/include/aidge/operator/ConvDepthWise.hpp
+++ b/include/aidge/operator/ConvDepthWise.hpp
@@ -85,7 +85,7 @@ public:
         bool associated = true;
         for (IOIndex_t i = 0; i < 3; ++i) {
             if (!getInput(i)) {
-                AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+                AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #{} should be associated with a Tensor", type(), i);
             }
             associated &= !(getInput(i)->empty());
         }
diff --git a/include/aidge/operator/FC.hpp b/include/aidge/operator/FC.hpp
index a73734ad2..e1f6f6bc1 100644
--- a/include/aidge/operator/FC.hpp
+++ b/include/aidge/operator/FC.hpp
@@ -84,7 +84,7 @@ public:
         bool associated = true;
         for (IOIndex_t i = 0; i < nbInputs(); ++i) {
             if (!getInput(i)) {
-                AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+                AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #{} should be associated with a Tensor", type(), i);
             }
             associated &= !(getInput(i)->empty());
         }
diff --git a/include/aidge/operator/MatMul.hpp b/include/aidge/operator/MatMul.hpp
index 3d80193be..1376624a3 100644
--- a/include/aidge/operator/MatMul.hpp
+++ b/include/aidge/operator/MatMul.hpp
@@ -72,7 +72,7 @@ public:
         bool associated = true;
         for (IOIndex_t i = 0; i < nbInputs(); ++i) {
             if (!getInput(i)) {
-                AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+                AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #{} should be associated with a Tensor", type(), i);
             }
             associated &= !(getInput(i)->empty());
         }
diff --git a/include/aidge/operator/MaxPooling.hpp b/include/aidge/operator/MaxPooling.hpp
index 467a69d73..b07fa38a4 100644
--- a/include/aidge/operator/MaxPooling.hpp
+++ b/include/aidge/operator/MaxPooling.hpp
@@ -78,7 +78,7 @@ public:
 
     void computeOutputDims() override final {
         if (!getInput(0)) {
-            AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+            AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #0 should be associated with a Tensor", type());
         }
         if (!(getInput(0)->empty())) {
             std::array<DimSize_t, DIM + 2> outputDims{};
diff --git a/include/aidge/operator/MetaOperator.hpp b/include/aidge/operator/MetaOperator.hpp
index 5955d860a..b38a2befe 100644
--- a/include/aidge/operator/MetaOperator.hpp
+++ b/include/aidge/operator/MetaOperator.hpp
@@ -56,6 +56,7 @@ public:
 
     void associateInput(const IOIndex_t inputIdx, const std::shared_ptr<Data>& data) override final {
         assert(strcmp(data->type(), Tensor::Type) == 0 && "input data must be of Tensor type");
+        AIDGE_ASSERT(inputIdx < mGraph->getOrderedInputs().size(), "associateInput(): inputIdx ({}) out of bound for MetaOperator", inputIdx);
 
         const auto& inputOp = mGraph->getOrderedInputs()[inputIdx];
         inputOp.first->getOperator()->associateInput(inputOp.second, data);
diff --git a/include/aidge/operator/Pad.hpp b/include/aidge/operator/Pad.hpp
index 56245dd2d..bb961295b 100644
--- a/include/aidge/operator/Pad.hpp
+++ b/include/aidge/operator/Pad.hpp
@@ -78,7 +78,7 @@ public:
         bool associated = true;
         for (IOIndex_t i = 0; i < nbInputs(); ++i) {
             if (!getInput(i)) {
-                AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+                AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #{} should be associated with a Tensor", type(), i);
             }
             associated &= !(getInput(i)->empty());
         }
diff --git a/include/aidge/utils/ErrorHandling.hpp b/include/aidge/utils/ErrorHandling.hpp
index 0abdb6bde..653a774b9 100644
--- a/include/aidge/utils/ErrorHandling.hpp
+++ b/include/aidge/utils/ErrorHandling.hpp
@@ -16,6 +16,7 @@
 #include <memory>
 
 #include <fmt/format.h>
+#include <fmt/ranges.h>
 
 #ifdef NO_EXCEPTION
 #define AIDGE_THROW_OR_ABORT(ex, ...) \
diff --git a/src/graph/GraphView.cpp b/src/graph/GraphView.cpp
index b4f8f8df4..328842cde 100644
--- a/src/graph/GraphView.cpp
+++ b/src/graph/GraphView.cpp
@@ -78,7 +78,13 @@ void Aidge::GraphView::save(std::string path, bool verbose, bool showProducers)
                       givenName.c_str());
         }
         else {
-            if ((node_ptr->type() != "Producer") || showProducers) {
+            if (node_ptr->type() == "Producer") {
+              if (showProducers) {
+                std::fprintf(fp, "%s_%s(%s):::producerCls\n", node_ptr->type().c_str(), namePtrTable[node_ptr].c_str(),
+                            givenName.c_str());
+              }
+            }
+            else {
                 std::fprintf(fp, "%s_%s(%s)\n", node_ptr->type().c_str(), namePtrTable[node_ptr].c_str(),
                             givenName.c_str());
             }
@@ -148,6 +154,7 @@ void Aidge::GraphView::save(std::string path, bool verbose, bool showProducers)
     std::fprintf(fp, "classDef outputCls fill:#ffa\n");
     std::fprintf(fp, "classDef externalCls fill:#ccc\n");
     std::fprintf(fp, "classDef rootCls stroke:#f00\n");
+    std::fprintf(fp, "classDef producerCls fill:#cbf\n");
     std::fprintf(fp, "\n");
     std::fclose(fp);
 }
diff --git a/src/operator/Memorize.cpp b/src/operator/Memorize.cpp
index 733d4d145..6e34c1a20 100644
--- a/src/operator/Memorize.cpp
+++ b/src/operator/Memorize.cpp
@@ -15,8 +15,10 @@
 const std::string Aidge::Memorize_Op::Type = "Memorize";
 
 void Aidge::Memorize_Op::computeOutputDims() {
-    if (!getInput(0) || !getInput(1)) {
-        AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+    for (size_t i = 0; i < 2; ++i) {
+        if (!getInput(i)) {
+            AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #{} should be associated with a Tensor", type(), i);
+        }
     }
 
     // Only require one of the input to have dims defined
diff --git a/src/operator/Mul.cpp b/src/operator/Mul.cpp
index fb82c1f75..54cda1162 100644
--- a/src/operator/Mul.cpp
+++ b/src/operator/Mul.cpp
@@ -35,6 +35,6 @@ void Aidge::Mul_Op::computeOutputDims() {
         mOutputs[0]->resize(getInput(0)->dims());
     }
     else if (!getInput(0)->empty() && !getInput(1)->empty()) {
-        AIDGE_THROW_OR_ABORT(std::runtime_error, "Incompatible input dimensions for Operator Mul");
+        AIDGE_THROW_OR_ABORT(std::runtime_error, "Incompatible input dimensions for Operator Mul: {} and {}", getInput(0)->dims(), getInput(1)->dims());
     }
 }
\ No newline at end of file
diff --git a/src/operator/Operator.cpp b/src/operator/Operator.cpp
index 32f172385..6959e044c 100644
--- a/src/operator/Operator.cpp
+++ b/src/operator/Operator.cpp
@@ -32,21 +32,21 @@ Aidge::Operator::~Operator() noexcept = default;
 ///////////////////////////////////////////////////////
 
 Aidge::NbElts_t Aidge::Operator::getNbRequiredData(const Aidge::IOIndex_t inputIdx) const {
-    AIDGE_ASSERT(mImpl != nullptr, "getNbRequiredData(): an implementation is required!");
+    AIDGE_ASSERT(mImpl != nullptr, "getNbRequiredData(): an implementation is required for {}!", type());
     return mImpl->getNbRequiredData(inputIdx);
 }
 
 Aidge::NbElts_t Aidge::Operator::getNbConsumedData(Aidge::IOIndex_t inputIdx) const {
-    AIDGE_ASSERT(mImpl != nullptr, "getNbConsumedData(): an implementation is required!");
+    AIDGE_ASSERT(mImpl != nullptr, "getNbConsumedData(): an implementation is required for {}!", type());
     return mImpl->getNbConsumedData(inputIdx);
 }
 
 Aidge::NbElts_t Aidge::Operator::getNbProducedData(Aidge::IOIndex_t outputIdx) const {
-    AIDGE_ASSERT(mImpl != nullptr, "getNbProducedData(): an implementation is required!");
+    AIDGE_ASSERT(mImpl != nullptr, "getNbProducedData(): an implementation is required for {}!", type());
     return mImpl->getNbProducedData(outputIdx);
 }
 void Aidge::Operator::updateConsummerProducer(){
-    AIDGE_ASSERT(mImpl != nullptr, "updateConsummerProducer(): an implementation is required!");
+    AIDGE_ASSERT(mImpl != nullptr, "updateConsummerProducer(): an implementation is required for {}!", type());
     mImpl->updateConsummerProducer();
 }
 
@@ -56,7 +56,7 @@ void Aidge::Operator::runHooks() const {
     }
 }
 void Aidge::Operator::forward() {
-    AIDGE_ASSERT(mImpl != nullptr, "forward(): an implementation is required!");
+    AIDGE_ASSERT(mImpl != nullptr, "forward(): an implementation is required for {}!", type());
     mImpl->forward();
     runHooks();
 }
diff --git a/src/operator/OperatorTensor.cpp b/src/operator/OperatorTensor.cpp
index f8eba813a..262f90788 100644
--- a/src/operator/OperatorTensor.cpp
+++ b/src/operator/OperatorTensor.cpp
@@ -117,7 +117,7 @@ void Aidge::OperatorTensor::computeOutputDims() {
     bool associated = (nbInputs() > 0); // do not compute anything if no input
     for (IOIndex_t i = 0; i < nbInputs(); ++i) {
         if (!getInput(i)) {
-            AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+            AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #{} should be associated with a Tensor", type(), i);
         }
         associated &= !(getInput(i)->empty());
     }
@@ -125,7 +125,9 @@ void Aidge::OperatorTensor::computeOutputDims() {
         const auto expectedDims =  getInput(0)->dims();
         for (std::size_t i = 1; i < nbInputs(); ++i) {
             if (expectedDims != getInput(i)->dims()) {
-                AIDGE_THROW_OR_ABORT(std::runtime_error, "Operator's inputs should have the same dimensions");
+                AIDGE_THROW_OR_ABORT(std::runtime_error,
+                    "{} operator's inputs should have the same dimensions: expected {} (input #0), given {} (input #{})",
+                    type(), expectedDims, getInput(i)->dims(), i);
             }
         }
         mOutputs[0]->resize(expectedDims);
diff --git a/src/operator/Slice.cpp b/src/operator/Slice.cpp
index 139e84b56..e4b072633 100644
--- a/src/operator/Slice.cpp
+++ b/src/operator/Slice.cpp
@@ -27,7 +27,7 @@ const std::string Aidge::Slice_Op::Type = "Slice";
 void Aidge::Slice_Op::computeOutputDims() {
     // check input have been associated
     if (!getInput(0) || (getInput(0)->empty())) {
-        AIDGE_THROW_OR_ABORT(std::runtime_error, "Every input should be associated with a Tensor");
+        AIDGE_THROW_OR_ABORT(std::runtime_error, "{}: input #0 should be associated with a Tensor", type());
     }
 
     DimSize_t nbAxes = this->template getAttr<SliceAttr::Axes>().size();
-- 
GitLab