IVGCVSW-7106 Incorrect Json format for some networks.

* ProfilingDetails assumed that every workload description included
  both tensors and parameters. This is not always the case.
* Modify ProfilingDetails::AddDetailsToString to check the next
  element to be printed before deciding to add a separator and new line.

Signed-off-by: Colm Donelan <colm.donelan@arm.com>
Change-Id: I2577b0e8a149d0a172ee12975e18b78238d8256e

2 files changed, 104 insertions, 28 deletions

diff --git a/src/armnn/ProfilingDetails.hpp b/src/armnn/ProfilingDetails.hpp
index 68ec00e7f8..e1f357b3f3 100644
--- a/src/armnn/ProfilingDetails.hpp
+++ b/src/armnn/ProfilingDetails.hpp
@@ -1,5 +1,5 @@
 //
-// Copyright © 2021 Arm Ltd and Contributors. All rights reserved.
+// Copyright © 2022 Arm Ltd and Contributors. All rights reserved.
 // SPDX-License-Identifier: MIT
 //
 
@@ -22,7 +22,7 @@ class ProfilingDetails : public JsonUtils
 {
 public:
     /// Constructor
-    ProfilingDetails() : JsonUtils(m_ProfilingDetails), m_DetailsExist(false), m_PrintSeparator(false)
+    ProfilingDetails() : JsonUtils(m_ProfilingDetails), m_DetailsExist(false)
     {}
 
     /// Destructor
@@ -30,7 +30,7 @@ public:
     {}
 
     /// Add to the ProfilingDetails
-    template<typename DescriptorType>
+    template <typename DescriptorType>
     void AddDetailsToString(const std::string& workloadName,
                             const DescriptorType& desc,
                             const WorkloadInfo& infos,
@@ -50,42 +50,54 @@ public:
         PrintNewLine();
         PrintTabs();
         m_ProfilingDetails << std::quoted("GUID") << ": " << std::quoted(std::to_string(guid));
-        PrintSeparator();
-        PrintNewLine();
+
+        // From this point onwards everything is potentially optional so we must be careful of separators and new lines.
 
         // Print tensor infos and related data types
-        PrintInfos(infos.m_InputTensorInfos, "Input");
+        if (!infos.m_InputTensorInfos.empty())
+        {
+            PrintSeparator();
+            PrintNewLine();
+            // Only add separator and new line if there is an output tensor info.
+            PrintInfos(infos.m_InputTensorInfos, "Input", !infos.m_OutputTensorInfos.empty());
+        }
 
-        PrintInfos(infos.m_OutputTensorInfos, "Output");
+        if (!infos.m_OutputTensorInfos.empty())
+        {
+            // Don't add a separator as we don't know what's next.
+            PrintInfos(infos.m_OutputTensorInfos, "Output", false);
+        }
 
-        if ( infos.m_BiasTensorInfo.has_value())
+        if (infos.m_BiasTensorInfo.has_value())
         {
-            PrintInfo(infos.m_BiasTensorInfo.value(), "Bias");
+            PrintSeparator();
+            PrintNewLine();
+            PrintInfo(infos.m_BiasTensorInfo.value(), "Bias", false);
         }
-        if ( infos.m_WeightsTensorInfo.has_value())
+
+        if (infos.m_WeightsTensorInfo.has_value())
         {
-            PrintInfo(infos.m_WeightsTensorInfo.value(), "Weights");
+            PrintSeparator();
+            PrintNewLine();
+            PrintInfo(infos.m_WeightsTensorInfo.value(), "Weights", false);
         }
-        if ( infos.m_ConvolutionMethod.has_value())
+
+        if (infos.m_ConvolutionMethod.has_value())
         {
+            PrintSeparator();
+            PrintNewLine();
             PrintTabs();
 
             m_ProfilingDetails << std::quoted("Convolution Method") << ": "
                                << std::quoted(infos.m_ConvolutionMethod.value());
-
-            PrintSeparator();
-            PrintNewLine();
         }
 
         ParameterStringifyFunction extractParams = [this](const std::string& name, const std::string& value) {
-            if (m_PrintSeparator)
-            {
-                PrintSeparator();
-                PrintNewLine();
-            }
+            // Always begin with a separator and new line.
+            PrintSeparator();
+            PrintNewLine();
             PrintTabs();
             m_ProfilingDetails << std::quoted(name) << " : " << std::quoted(value);
-            m_PrintSeparator = true;
         };
 
         StringifyLayerParameters<DescriptorType>::Serialize(extractParams, desc);
@@ -94,7 +106,6 @@ public:
         PrintFooter();
 
         m_DetailsExist = true;
-        m_PrintSeparator = false;
     }
 
     /// Get the ProfilingDetails
@@ -111,13 +122,13 @@ public:
 
 private:
     // Print tensor infos and related data types
-    void PrintInfo(const TensorInfo& info, const std::string& ioString)
+    void PrintInfo(const TensorInfo& info, const std::string& ioString, bool addSeparator = true)
     {
         const std::vector<TensorInfo> infoVect{ info };
-        PrintInfos(infoVect, ioString);
+        PrintInfos(infoVect, ioString, addSeparator);
     }
 
-    void PrintInfos(const std::vector<TensorInfo>& infos, const std::string& ioString)
+    void PrintInfos(const std::vector<TensorInfo>& infos, const std::string& ioString, bool addSeparator = true)
     {
         for ( size_t i = 0; i < infos.size(); i++ )
         {
@@ -158,15 +169,17 @@ private:
             // Close out the scope
             PrintNewLine();
             PrintFooter();
-            PrintSeparator();
-            PrintNewLine();
+            if (addSeparator)
+            {
+                PrintSeparator();
+                PrintNewLine();
+            }
         }
     }
 
     /// Stores ProfilingDetails
     std::ostringstream m_ProfilingDetails;
     bool m_DetailsExist;
-    bool m_PrintSeparator;
 
 };
 
diff --git a/src/armnn/test/ProfilerTests.cpp b/src/armnn/test/ProfilerTests.cpp
index 6cb8d7fae6..6bccd5c97f 100644
--- a/src/armnn/test/ProfilerTests.cpp
+++ b/src/armnn/test/ProfilerTests.cpp
@@ -12,6 +12,7 @@
 
 #include <Profiling.hpp>
 #include <armnn/Optional.hpp>
+#include <armnnUtils/TensorUtils.hpp>
 
 namespace armnn
 {
@@ -107,6 +108,68 @@ TEST_CASE("RegisterUnregisterProfilerMultipleThreads")
     }
 }
 
+TEST_CASE("LayerWorkloadConstructorWithEmptyWorkloadInfo")
+{
+    // Calling AddDetailsToString with a descriptor that contains no tensors or parameters results
+    // in an invalid piece of JSON as the function assumes there will be something after the input
+    // and output tensors are printed and leaves a hanging ','
+    // This test validates that the fix for that continues to work.
+    armnn::ProfilingDetails classOnTest;
+    armnn::ArgMinMaxDescriptor descriptor;
+    armnn::WorkloadInfo workloadInfo;
+    arm::pipe::ProfilingGuid guid;
+    classOnTest.AddDetailsToString("NeonArgMinMaxWorkload_Construct", descriptor, workloadInfo, guid);
+    std::string result = classOnTest.GetProfilingDetails();
+    // Make sure the string ends with a "GUID": "0"\n}"
+    REQUIRE(result.find("GUID\": \"0\"\n}") != std::string::npos);
+}
+
+TEST_CASE("LayerWorkloadConstructorWithWorkloadInfoOnlyInputTensor")
+{
+    // Calling AddDetailsToString with a descriptor that contains a single inout tensor and no output
+    // tensor or parameters results in an invalid piece of JSON as the function assumes there will be
+    // something after the input and output tensors are printed and leaves a hanging ','
+    // This test validates that the fix for that continues to work.
+
+    armnn::ProfilingDetails classOnTest;
+    armnn::ArgMinMaxDescriptor descriptor;
+    armnn::WorkloadInfo workloadInfo;
+    arm::pipe::ProfilingGuid guid;
+    armnn::TensorInfo inputTensorInfo =
+        armnnUtils::GetTensorInfo(1, 1, 1, 1, armnn::DataLayout::NCHW, armnn::DataType::Float32);
+    workloadInfo.m_InputTensorInfos.push_back(inputTensorInfo);
+
+    classOnTest.AddDetailsToString("NeonArgMinMaxWorkload_Construct", descriptor, workloadInfo, guid);
+    std::string result = classOnTest.GetProfilingDetails();
+    // Make sure the string ends with a "Num Dims\": \"4\"\n\t}\n}"
+    REQUIRE(result.find("Num Dims\": \"4\"\n\t}\n}") != std::string::npos);
+}
+
+TEST_CASE("LayerWorkloadConstructorWithWorkloadInfoInputAndOutputTensorNoParameters")
+{
+    // Calling AddDetailsToString with a descriptor that contains no tensors or parameters results
+    // in an invalid piece of JSON as the function assumes there will be something after the input
+    // and output tensors are printed and leaves a hanging ','
+    // This test validates that the fix for that continues to work.
+    armnn::ProfilingDetails classOnTest;
+    armnn::ArgMinMaxDescriptor descriptor;
+    armnn::WorkloadInfo workloadInfo;
+    arm::pipe::ProfilingGuid guid;
+    armnn::TensorInfo inputTensorInfo =
+        armnnUtils::GetTensorInfo(1, 1, 1, 1, armnn::DataLayout::NCHW, armnn::DataType::Float32);
+    workloadInfo.m_InputTensorInfos.push_back(inputTensorInfo);
+
+    // We'll make the output tensrinfo have 5 dimensions to make errors easier to detect.
+    armnn::TensorInfo outputTensorInfo =
+        armnnUtils::GetTensorInfo(1, 1, 1, 1, 1, armnn::DataLayout::NCDHW, armnn::DataType::Float32);
+    workloadInfo.m_OutputTensorInfos.push_back(outputTensorInfo);
+
+    classOnTest.AddDetailsToString("NeonArgMinMaxWorkload_Construct", descriptor, workloadInfo, guid);
+    std::string result = classOnTest.GetProfilingDetails();
+    // Make sure the string ends with a "Num Dims\": \"4\"\n\t}\n}"
+    REQUIRE(result.find("Num Dims\": \"5\"\n\t}\n}") != std::string::npos);
+}
+
 TEST_CASE("ProfilingMacros")
 {
     // Get a reference to the profiler manager.