From 0cf01dce414db3ce9595a438cf3c3f10dd938450 Mon Sep 17 00:00:00 2001 From: Matthew Bentham Date: Tue, 30 Jul 2019 08:24:12 +0000 Subject: IVGCVSW-3581 Fix AddCopyLayers and associated tests Take a copy of the MemoryStrategies for a layer before inserting new connections. Use the copy when looking up the original MemoryStrategies during the graph transformation. Fix the unit tests for AddCopyLayers to have cases where copies are needed. Fix the validation for clarity and correctness - was previously comparing Layers by pointer when it should have been by name (as it was comparing with a cloned graph). Change-Id: Ie282dc11913e977b8151ce1ad8bfba5e11617d40 Signed-off-by: Matthew Bentham --- src/armnn/Graph.cpp | 16 ++++++++++++++-- src/armnn/Layer.hpp | 1 + src/armnn/NetworkUtils.cpp | 3 ++- src/armnn/test/GraphTests.cpp | 44 +++++++++++++++++++++++++++++-------------- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/armnn/Graph.cpp b/src/armnn/Graph.cpp index e521623737..9e00f5ec01 100644 --- a/src/armnn/Graph.cpp +++ b/src/armnn/Graph.cpp @@ -285,12 +285,13 @@ void Graph::AddCopyLayers(std::map> { OutputSlot& srcOutputSlot = srcLayer->GetOutputSlot(srcOutputIndex); const std::vector srcConnections = srcOutputSlot.GetConnections(); + const std::vector srcMemoryStrategies = srcOutputSlot.GetMemoryStrategies(); for (unsigned int srcConnectionIndex = 0; srcConnectionIndex < srcConnections.size(); srcConnectionIndex++) { InputSlot* dstInputSlot = srcConnections[srcConnectionIndex]; BOOST_ASSERT(dstInputSlot); - auto strategy = srcOutputSlot.GetMemoryStrategyForConnection(srcConnectionIndex); + MemoryStrategy strategy = srcMemoryStrategies[srcConnectionIndex]; BOOST_ASSERT_MSG(strategy != MemoryStrategy::Undefined, "Undefined memory strategy found while adding copy layers for compatibility"); @@ -339,8 +340,19 @@ void Graph::AddCopyLayers(std::map> copyOutputSlot.SetTensorHandleFactory(ITensorHandleFactory::LegacyFactoryId); } + // The output strategy of a copy layer is always DirectCompatibility. copyOutputSlot.SetMemoryStrategy(0, MemoryStrategy::DirectCompatibility); - srcOutputSlot.SetMemoryStrategy(srcConnectionIndex, MemoryStrategy::DirectCompatibility); + + // Recalculate the connection index on the previous layer as we have just inserted into it. + const std::vector& newSourceConnections = srcOutputSlot.GetConnections(); + long newSrcConnectionIndex = std::distance(newSourceConnections.begin(), + std::find(newSourceConnections.begin(), + newSourceConnections.end(), + ©Layer->GetInputSlot(0))); + + // The input strategy of a copy layer is always DirectCompatibilty. + srcOutputSlot.SetMemoryStrategy(boost::numeric_cast(newSrcConnectionIndex), + MemoryStrategy::DirectCompatibility); } } } diff --git a/src/armnn/Layer.hpp b/src/armnn/Layer.hpp index 1ddbc00bc7..b90d040475 100644 --- a/src/armnn/Layer.hpp +++ b/src/armnn/Layer.hpp @@ -123,6 +123,7 @@ public: void Disconnect(InputSlot& slot); const std::vector& GetConnections() const { return m_Connections; } + const std::vector& GetMemoryStrategies() const { return m_MemoryStrategies; } bool ValidateTensorShape(const TensorShape& shape) const; diff --git a/src/armnn/NetworkUtils.cpp b/src/armnn/NetworkUtils.cpp index 66940e4eb5..a3760a9c6b 100644 --- a/src/armnn/NetworkUtils.cpp +++ b/src/armnn/NetworkUtils.cpp @@ -86,7 +86,7 @@ std::vector InsertDebugLayerAfter(Graph& graph, Layer& layer) debugLayers.reserve(layer.GetNumOutputSlots()); // Connect a DebugLayer to each output slot of the layer - for (auto&& outputSlot = layer.BeginOutputSlots(); outputSlot != layer.EndOutputSlots(); ++outputSlot) + for (auto outputSlot = layer.BeginOutputSlots(); outputSlot != layer.EndOutputSlots(); ++outputSlot) { const std::string debugName = std::string("DebugLayerAfter") + layer.GetNameStr(); @@ -94,6 +94,7 @@ std::vector InsertDebugLayerAfter(Graph& graph, Layer& layer) graph.InsertNewLayer(*outputSlot, debugName.c_str()); // Sets output tensor info for the debug layer. + BOOST_ASSERT(debugLayer->GetInputSlot(0).GetConnectedOutputSlot() == &(*outputSlot)); TensorInfo debugInfo = debugLayer->GetInputSlot(0).GetConnectedOutputSlot()->GetTensorInfo(); debugLayer->GetOutputSlot().SetTensorInfo(debugInfo); diff --git a/src/armnn/test/GraphTests.cpp b/src/armnn/test/GraphTests.cpp index ae5ff2232b..7950ec49f4 100644 --- a/src/armnn/test/GraphTests.cpp +++ b/src/armnn/test/GraphTests.cpp @@ -342,13 +342,14 @@ static void TestGraphAfterAddingCopyLayers(const armnn::Graph& graph, const armn if (srcLayer == nullptr || dstLayer == nullptr) { BOOST_ERROR("At least one of the two ends of a new edge (" << edge.first << ", " << edge.second << ") " - "introduced after adding copy layers to a graph correspond is not known to the graph"); + "introduced after adding copy layers to a graph " + "correspond to a layer not known to the graph"); continue; } // One and only one of the two layers referenced by the edge should be present in the original graph. - const bool srcLayerInOrigGraph = GraphHasNamedLayer(origGraph, edge.first->GetNameStr()); - const bool dstLayerInOrigGraph = GraphHasNamedLayer(origGraph, edge.second->GetNameStr()); + const bool srcLayerInOrigGraph = GraphHasNamedLayer(origGraph, srcLayer->GetNameStr()); + const bool dstLayerInOrigGraph = GraphHasNamedLayer(origGraph, dstLayer->GetNameStr()); if (srcLayerInOrigGraph == dstLayerInOrigGraph) { @@ -363,7 +364,7 @@ static void TestGraphAfterAddingCopyLayers(const armnn::Graph& graph, const armn continue; } - const armnn::Layer* copyLayer = srcLayerInOrigGraph ? edge.second : edge.first; + const armnn::Layer* copyLayer = srcLayerInOrigGraph ? dstLayer : srcLayer; const armnn::Layer* nonCopyLayer = srcLayerInOrigGraph ? srcLayer : dstLayer; // Finds all edges connecting the copy layer to other layers. @@ -412,10 +413,18 @@ static void TestGraphAfterAddingCopyLayers(const armnn::Graph& graph, const armn // There must exist an edge connecting both layers directly in the original graph. { - const armnn::Layer* origEdgeN1 = srcLayerInOrigGraph ? nonCopyLayer : adjLayer; - const armnn::Layer* origEdgeN2 = srcLayerInOrigGraph ? adjLayer : nonCopyLayer; - auto origEdgeIter = std::find(origEdges.begin(), origEdges.end(), - Edge(origEdgeN1, origEdgeN2)); + const armnn::Layer* origEdgeSrc = srcLayerInOrigGraph ? nonCopyLayer : adjLayer; + const armnn::Layer* origEdgeDst = srcLayerInOrigGraph ? adjLayer : nonCopyLayer; + + auto origEdgeIter = origEdges.begin(); + for (; origEdgeIter != origEdges.end(); origEdgeIter++) + { + if (origEdgeIter->first->GetNameStr() == origEdgeSrc->GetNameStr() && + origEdgeIter->second->GetNameStr() == origEdgeDst->GetNameStr()) + { + break; + } + } if (origEdgeIter != origEdges.end()) { @@ -438,6 +447,10 @@ static void TestGraphAfterAddingCopyLayers(const armnn::Graph& graph, const armn struct CopyLayersFixture { CopyLayersFixture() + { + } + + void InitialiseTestGraph() { using namespace armnn; using namespace std; @@ -452,7 +465,7 @@ struct CopyLayersFixture inputLayer->GetOutputSlot(0).Connect(convLayer1->GetInputSlot(0)); Layer* const convLayer2 = AddLayer(convolutionDefaults, "conv2"); - convLayer2->SetBackendId(Compute::CpuRef); + convLayer2->SetBackendId(Compute::CpuAcc); convLayer1->GetOutputSlot(0).Connect(convLayer2->GetInputSlot(0)); @@ -476,18 +489,19 @@ struct CopyLayersFixture actLayer->GetOutputSlot(0).Connect(softmaxLayer->GetInputSlot(0)); Layer* const outputLayer = AddLayer(0, "output"); - outputLayer->SetBackendId(armnn::Compute::CpuRef); + outputLayer->SetBackendId(armnn::Compute::CpuAcc); softmaxLayer->GetOutputSlot(0).Connect(outputLayer->GetInputSlot(0)); - // Set the memory strategies + // Set the memory strategies - for this test should be DirectCompatibility for same backends, + // and CopyToTarget for different backends inputLayer->GetOutputSlot(0).SetMemoryStrategy(0, MemoryStrategy::DirectCompatibility); - convLayer1->GetOutputSlot(0).SetMemoryStrategy(0, MemoryStrategy::DirectCompatibility); + convLayer1->GetOutputSlot(0).SetMemoryStrategy(0, MemoryStrategy::CopyToTarget); convLayer1->GetOutputSlot(0).SetMemoryStrategy(1, MemoryStrategy::DirectCompatibility); - convLayer2->GetOutputSlot(0).SetMemoryStrategy(0, MemoryStrategy::DirectCompatibility); + convLayer2->GetOutputSlot(0).SetMemoryStrategy(0, MemoryStrategy::CopyToTarget); concatLayer->GetOutputSlot(0).SetMemoryStrategy(0, MemoryStrategy::DirectCompatibility); actLayer->GetOutputSlot(0).SetMemoryStrategy(0, MemoryStrategy::DirectCompatibility); - softmaxLayer->GetOutputSlot(0).SetMemoryStrategy(0, MemoryStrategy::DirectCompatibility); + softmaxLayer->GetOutputSlot(0).SetMemoryStrategy(0, MemoryStrategy::CopyToTarget); } armnn::TensorInfo m_TensorDesc; @@ -513,6 +527,7 @@ private: BOOST_FIXTURE_TEST_CASE(AddCopyLayers, CopyLayersFixture) { + InitialiseTestGraph(); const armnn::Graph origGraph(m_Graph); m_Graph.AddCopyLayers(m_Backends, m_FactoryRegistry); @@ -521,6 +536,7 @@ BOOST_FIXTURE_TEST_CASE(AddCopyLayers, CopyLayersFixture) BOOST_FIXTURE_TEST_CASE(AddCopyLayersSeveralTimes, CopyLayersFixture) { + InitialiseTestGraph(); m_Graph.AddCopyLayers(m_Backends, m_FactoryRegistry); // Calling AddCopyLayers() several times should not change the connections. -- cgit v1.2.1