[tmva][sofie] Fix sume bugs in handling dynamic shapes

Fix some bugs in dealing with the dynamic shapes. Parathesis must be correctly added
Fix also a bug in avoid two times broadcasting of constant input tensors in the Baniry operator

Remove the commented code as suggested by Sanjiban's review

Author: lmoneta

tmva/sofie/inc/TMVA/ROperator_BasicBinary.hxx

@@ -141,6 +141,7 @@ public:
                   // Update the data and the shape of A
                   model.AddConstantTensor(fNBroadcastedA, model.GetTensorType(fNA), fShapeY, broadcastedData);
                   fShapeA = fShapeY;
+                  fDimShapeA = ConvertShapeToDim(fShapeA);
                } else {
                   // Add an intermediate tensor for broadcasting A
                   model.AddIntermediateTensor(fNBroadcastedA, model.GetTensorType(fNA), fShapeY);
@@ -166,6 +167,7 @@ public:
                                << std::endl;
                   model.AddConstantTensor(fNBroadcastedB, model.GetTensorType(fNB), fShapeY, broadcastedData);
                   fShapeB = fShapeY;
+                  fDimShapeB = ConvertShapeToDim(fShapeB);
                } else {
                   // Add an intermediate tensor for broadcasting B
                   model.AddIntermediateTensor(fNBroadcastedB, model.GetTensorType(fNB), fShapeY);
@@ -314,14 +316,15 @@ public:
       }
       // Broadcast A if it's uninitialized
       // use broadcasting function where we pass an already allocated tensor to minimize memory allocations
-      if (!fNBroadcastedA.empty()) {
+      // in case of A is  constant tensor is automatically broadcasted in Initialize() then shapeA == shapeY
+      if (!fNBroadcastedA.empty() && (fDimShapeA != fDimShapeY)) {
          out << SP << SP << "// Broadcasting uninitialized tensor " << fNA << "\n";
          out << SP << SP  << "TMVA::Experimental::SOFIE::UTILITY::UnidirectionalBroadcast<" << typeName << ">(tensor_" << fNA << ", "
                   << ConvertDimShapeToString(fDimShapeA) << ", " << ConvertDimShapeToString(fDimShapeY)
                   << ", fTensor_" << fNBroadcastedA << ");\n";
       }
       // Broadcast B if it's uninitialized
-      if (!fNBroadcastedB.empty()) {
+      if (!fNBroadcastedB.empty() && (fDimShapeB != fDimShapeY)) {
          out << SP << SP << "// Broadcasting uninitialized tensor " << fNB << "\n";
          out << SP << SP << "TMVA::Experimental::SOFIE::UTILITY::UnidirectionalBroadcast<" << typeName << ">(tensor_" << fNB << ", "
                   << ConvertDimShapeToString(fDimShapeB) << ", " << ConvertDimShapeToString(fDimShapeY)

tmva/sofie/inc/TMVA/ROperator_Concat.hxx

@@ -150,25 +150,13 @@
                      }
 
                   }
+                  // add parenthesis in case is an expression
+                  if (concat_dim.isParam && concat_dim.dim == static_cast<size_t>(-1))
+                     concat_dim =  Dim{ std::string("(") + concat_dim.GetVal() +  std::string(")"), concat_dim.dim };
                }
 
                // output shape for concatenated axis
                ret[fAxis] = Dim{concat_dim};
-               // //ret[0] = inputs[0];
-               // // check if concat_dim is an integer
-               // // case like "2+n" can be converted to an integer so need to check the length
-               // size_t pos = 0;
-               // try {
-               //    i_concat_dim = std::stoi(concat_dim, &pos);
-               //    if (pos == concat_dim.length())
-               //       ret[fAxis] = Dim{i_concat_dim}; // dimension is integer
-               //    else {
-               //       // check if a composite expression
-               //       ret[fAxis] = Dim{concat_dim};
-               // }
-               // catch (std::invalid_argument const& ex) {
-
-               // }
 
             }
             // case of stacking (not supported yet)
@@ -241,7 +229,7 @@
                         for (size_t i = 0; i < inputData.size(); i++)
                            inputData[i] = Dim{ static_cast<size_t>(intData[i])};
                      }
-                     std::cout << "concatanating input data " << inputLength << "  " << inputData[0] << std::endl;
+                     std::cout << "concatenating input data " << inputLength << "  " << inputData[0] << std::endl;
                      std::copy(inputData.begin(), inputData.end(), outputData.begin() + offset );
                      offset += inputLength;
                   }

tmva/sofie/inc/TMVA/ROperator_Reshape.hxx

@@ -121,7 +121,7 @@ public:
                         output_shape[i] = Dim{std::string("(") + ConvertDimShapeToLength(reduced_input) + ")", static_cast<size_t>(-1)};
                   }
                   if (!canSimplify)
-                     output_shape[i] = Dim{std::string("(") + input_length + " / (" + tmp_length + ")", static_cast<size_t>(-1)};
+                     output_shape[i] = Dim{std::string("(") + input_length + " / (" + tmp_length + "))", static_cast<size_t>(-1)};
                }
 
                break; // cannot have more than -1

tmva/sofie/inc/TMVA/ROperator_Transpose.hxx

@@ -147,7 +147,8 @@ public:
       //......
       // and we have j_k = i_fAttrPerm[k]
       // since we are using consecutive writes we should find the inverse of fAttrPerm
-      out << SP << "///------- Transpose operator\n" << std::endl;
+      out << SP << "///------- Transpose operator " << OpName << ConvertDimShapeToString(fShapeData)
+                  << " --> " << ConvertDimShapeToString(fShapeOutput) << std::endl;
       out << SP << "for (size_t id = 0; id < " << length << " ; id++){\n";
       out << SP << SP << "tensor_" << fNOutput << "[id] = tensor_" << fNData << "[ ";
       // compute output j indices
@@ -156,9 +157,9 @@ public:
          if (k == 0)
             i_out[k] = "id";
          else
-            i_out[k] = "(id % " + outStrides[k-1].GetVal() + ")";
+            i_out[k] = "(id % (" + outStrides[k-1].GetVal() + "))";
          if (k < dim-1)
-            i_out[k] += " / " + outStrides[k].GetVal();
+            i_out[k] += " / (" + outStrides[k].GetVal() + ")";
       }
       // use now them for input tensors
       // need to invert the fAttrPerm[k]