From a1e1ee8696d842a0e7b660865bd0317d6e509ece Mon Sep 17 00:00:00 2001 From: Colm Donelan Date: Mon, 8 Aug 2022 21:13:34 +0100 Subject: 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 Change-Id: I2577b0e8a149d0a172ee12975e18b78238d8256e --- src/armnn/ProfilingDetails.hpp | 69 ++++++++++++++++++++++++---------------- src/armnn/test/ProfilerTests.cpp | 63 ++++++++++++++++++++++++++++++++++++ 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 + template 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::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 infoVect{ info }; - PrintInfos(infoVect, ioString); + PrintInfos(infoVect, ioString, addSeparator); } - void PrintInfos(const std::vector& infos, const std::string& ioString) + void PrintInfos(const std::vector& 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 #include +#include 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. -- cgit v1.2.1