[graph] Graph iterator for sorted and unsorted

Updated graph iteartor to run over unsorted list of nodes
if the graph is not yet sorted. If the graph is sorted,
the iterator iterates over sorted nodes.

**Self evaluation:**
1. Build test: [x]Passed [ ]Failed [ ]Skipped
2. Run test: [x]Passed [ ]Failed [ ]Skipped

Signed-off-by: Parichay Kapoor <pk.kapoor@samsung.com>

diff --git a/nntrainer/graph/graph_core.cpp b/nntrainer/graph/graph_core.cpp
index 49f5b6b..c183cf3 100644 (file)
--- a/nntrainer/graph/graph_core.cpp
+++ b/nntrainer/graph/graph_core.cpp
@@ -102,6 +102,9 @@ void GraphCore::topologicalSort() {
     Sorted.push_back(dfs_stack.top());
     dfs_stack.pop();
   }
+
+  if (Sorted.size() != node_list.size())
+    throw std::runtime_error("Internal error in topologicalSort");
 }
 
 const std::shared_ptr<GraphNode> &
@@ -116,19 +119,6 @@ GraphCore::getNode(const std::string &name) const {
   throw std::invalid_argument(ss.str());
 }
 
-std::vector<std::shared_ptr<GraphNode>> GraphCore::getNodes() const {
-  std::vector<std::shared_ptr<GraphNode>> ret;
-  if (!Sorted.empty()) {
-    std::transform(Sorted.begin(), Sorted.end(), std::back_inserter(ret),
-                   [](auto const &elem) { return elem; });
-  } else {
-    std::transform(node_list.begin(), node_list.end(), std::back_inserter(ret),
-                   [](auto const &elem) { return elem; });
-  }
-
-  return ret;
-}
-
 void GraphCore::addNode(std::shared_ptr<GraphNode> node, bool ensure_name) {
   /** Ensure that the node has a name and is unique */
   if (ensure_name)

diff --git a/nntrainer/graph/graph_core.h b/nntrainer/graph/graph_core.h
index 2444036..112ad05 100644 (file)
--- a/nntrainer/graph/graph_core.h
+++ b/nntrainer/graph/graph_core.h
@@ -102,15 +102,6 @@ public:
   const std::shared_ptr<GraphNode> &getNode(const std::string &name) const;
 
   /**
-   * @brief getter all the node nodes in the model
-   * @retval node nodes
-   * @note these node nodes will be in sorted order if the model is compiled,
-   * otherwise the order is the order of addition of node nodes in the model.
-   * TODO: deprecate this
-   */
-  std::vector<std::shared_ptr<GraphNode>> getNodes() const;
-
-  /**
    * @brief     join passed graph into the existing graph model
    * @param[in] graph graph to be added/to extend
    * @param[in] prefix prefix added to names of nodes from this graph
@@ -126,13 +117,15 @@ public:
   /**
    * @brief     get begin iterator for the forwarding
    * @retval    const iterator marking the begin of forwarding
-   * TODO:      iterate over node_list if sorted is not available
    */
   template <
     typename T = GraphNode,
     std::enable_if_t<std::is_base_of<GraphNode, T>::value, T> * = nullptr>
   inline graph_iterator<T> begin() {
-    return graph_iterator<T>(&(*Sorted.begin()));
+    if (Sorted.empty())
+      return graph_iterator<T>(&(*node_list.begin()));
+    else
+      return graph_iterator<T>(&(*Sorted.begin()));
   }
 
   /**
@@ -143,7 +136,10 @@ public:
     typename T = GraphNode,
     std::enable_if_t<std::is_base_of<GraphNode, T>::value, T> * = nullptr>
   inline graph_iterator<T> end() {
-    return graph_iterator<T>(&(*Sorted.end()));
+    if (Sorted.empty())
+      return graph_iterator<T>(&(*node_list.end()));
+    else
+      return graph_iterator<T>(&(*Sorted.end()));
   }
 
   /**
@@ -154,7 +150,10 @@ public:
     typename T = GraphNode,
     std::enable_if_t<std::is_base_of<GraphNode, T>::value, T> * = nullptr>
   inline graph_const_iterator<T> cbegin() const {
-    return graph_const_iterator<T>(&(*Sorted.cbegin()));
+    if (Sorted.empty())
+      return graph_const_iterator<T>(&(*node_list.cbegin()));
+    else
+      return graph_const_iterator<T>(&(*Sorted.cbegin()));
   }
 
   /**
@@ -165,7 +164,10 @@ public:
     typename T = GraphNode,
     std::enable_if_t<std::is_base_of<GraphNode, T>::value, T> * = nullptr>
   inline graph_const_iterator<T> cend() const {
-    return graph_const_iterator<T>(&(*Sorted.cend()));
+    if (Sorted.empty())
+      return graph_const_iterator<T>(&(*node_list.cend()));
+    else
+      return graph_const_iterator<T>(&(*Sorted.cend()));
   }
 
   /**

diff --git a/nntrainer/graph/graph_node.h b/nntrainer/graph/graph_node.h
index 0724346..6e75dad 100644 (file)
--- a/nntrainer/graph/graph_node.h
+++ b/nntrainer/graph/graph_node.h
@@ -90,7 +90,8 @@ public:
  *
  * @note    GraphNodeType is to enable for both GraphNode and const GraphNode
  */
-template <typename LayerNodeType, typename GraphNodeType>
+template <typename LayerNodeType, typename LayerNodePtrType,
+          typename GraphNodeType>
 class GraphNodeIterator
   : public std::iterator<std::random_access_iterator_tag, GraphNodeType> {
   GraphNodeType *p; /** underlying object of GraphNode */
@@ -105,11 +106,11 @@ public:
    * @note    value_type, pointer and reference are different from standard
    * iterator
    */
-  typedef std::shared_ptr<LayerNodeType> value_type;
+  typedef LayerNodePtrType value_type;
   typedef std::random_access_iterator_tag iterator_category;
   typedef std::ptrdiff_t difference_type;
-  typedef std::shared_ptr<LayerNodeType> *pointer;
-  typedef std::shared_ptr<LayerNodeType> &reference;
+  typedef LayerNodePtrType *pointer;
+  typedef LayerNodePtrType &reference;
 
   /**
    * @brief Construct a new Graph Node Iterator object
@@ -305,28 +306,31 @@ public:
  */
 template <class LayerNodeType>
 using graph_iterator =
-  GraphNodeIterator<LayerNodeType, std::shared_ptr<GraphNode>>;
+  GraphNodeIterator<LayerNodeType, std::shared_ptr<LayerNodeType>,
+                    std::shared_ptr<GraphNode>>;
 
 /**
  * @brief     Iterators to traverse the graph
  */
 template <class LayerNodeType>
-using graph_reverse_iterator = GraphNodeReverseIterator<
-  GraphNodeIterator<LayerNodeType, std::shared_ptr<GraphNode>>>;
+using graph_reverse_iterator = GraphNodeReverseIterator<GraphNodeIterator<
+  LayerNodeType, std::shared_ptr<LayerNodeType>, std::shared_ptr<GraphNode>>>;
 
 /**
  * @brief     Iterators to traverse the graph
  */
 template <class LayerNodeType>
 using graph_const_iterator =
-  GraphNodeIterator<const LayerNodeType, const std::shared_ptr<GraphNode>>;
+  GraphNodeIterator<LayerNodeType, const std::shared_ptr<LayerNodeType>,
+                    const std::shared_ptr<GraphNode>>;
 
 /**
  * @brief     Iterators to traverse the graph
  */
 template <class LayerNodeType>
 using graph_const_reverse_iterator = GraphNodeReverseIterator<
-  GraphNodeIterator<const LayerNodeType, const std::shared_ptr<GraphNode>>>;
+  GraphNodeIterator<LayerNodeType, const std::shared_ptr<LayerNodeType>,
+                    const std::shared_ptr<GraphNode>>>;
 
 } // namespace nntrainer
 #endif // __GRAPH_NODE_H__

diff --git a/nntrainer/graph/network_graph.cpp b/nntrainer/graph/network_graph.cpp
index 2eb21dd..6da3736 100644 (file)
--- a/nntrainer/graph/network_graph.cpp
+++ b/nntrainer/graph/network_graph.cpp
@@ -62,20 +62,17 @@ int NetworkGraph::compile(const LossType loss_type) {
 
 void NetworkGraph::updateConnectionName(const std::string &from,
                                         const std::string &to) {
-  const std::vector<std::shared_ptr<GraphNode>> &node_list = graph.getNodes();
-  for (unsigned int i = 0; i < node_list.size(); ++i) {
-    auto &layer = node_list[i];
-    if (istrequal(layer->getName(), to))
+  for (auto const &gnode : graph) {
+    if (istrequal(gnode->getName(), to))
       continue;
-    LNODE(node_list[i])->updateInputLayers(from, to);
+    LNODE(gnode)->updateInputLayers(from, to);
   }
 }
 
 void NetworkGraph::addDefaultInputLayers() {
-  const std::vector<std::shared_ptr<GraphNode>> &node_list = graph.getNodes();
-  for (unsigned int i = 1; i < node_list.size(); ++i) {
-    auto layer = LNODE(node_list[i]);
-    auto prev_layer = LNODE(node_list[i - 1]);
+  for (auto iter = cbegin() + 1; iter != cend(); iter++) {
+    auto layer = *iter;
+    auto prev_layer = *(iter - 1);
     if (layer->getNumInputs() == 0) {
       layer->addInputLayers(prev_layer->getName());
     }
@@ -305,13 +302,11 @@ int NetworkGraph::addLossLayer(const LossType loss_type) {
 
 void NetworkGraph::setOutputLayers() {
 
-  const std::vector<std::shared_ptr<GraphNode>> &node_list = graph.getNodes();
-
   size_t last_layer_count = 0;
-  for (unsigned int idx = 0; idx < graph.size(); ++idx) {
-    auto layer_idx = LNODE(node_list[idx]);
-    for (unsigned int i = 0; i < graph.size(); ++i) {
-      auto layer_i = LNODE(node_list[i]);
+  for (auto const gnode_idx : graph) {
+    auto layer_idx = LNODE(gnode_idx);
+    for (auto const gnode_i : graph) {
+      auto layer_i = LNODE(gnode_i);
       if (istrequal(layer_i->getName(), layer_idx->getName()))
         continue;
       for (unsigned int j = 0; j < layer_i->getNumInputs(); ++j) {
@@ -348,8 +343,8 @@ void NetworkGraph::setOutputLayers() {
       "Error: Multiple last layers in the model not supported");
   }
 
-  for (unsigned int idx = 0; idx < graph.size(); ++idx) {
-    if (LNODE(node_list[idx])->getNumOutputs() == 0)
+  for (auto const &gnode : graph) {
+    if (LNODE(gnode)->getNumOutputs() == 0)
       throw std::runtime_error("There is un-connected node");
   }
 }
@@ -400,13 +395,14 @@ int NetworkGraph::realizeGraph() {
 
   addDefaultInputLayers();
 
-  /** This loop modifes the graph. Get the size of graph preemptively. */
-  size_t num_nodes = graph.size();
-  std::vector<std::shared_ptr<GraphNode>> node_list = graph.getNodes();
-
-  for (unsigned int i = 0; i < num_nodes; ++i) {
-    auto const &lnode = LNODE(node_list[i]);
-    LayerV1 &l = *lnode->getObject();
+  /**
+   * invariant: the new realized nodes are added to the end,
+   * otherwise this iteration becomes invalid. So, every iteration must be fresh
+   * iterator as vector resize invalidates all the iterators.
+   */
+  for (unsigned int i = 0; i < graph.size(); ++i) {
+    auto const &lnode = LNODE(*(cbegin() + i));
+    auto &l = *lnode->getObject();
     ml_logd("layer name: %s", lnode->getName().c_str());
 
     /** If a layer does not has input nodes, then it must have input dimension
@@ -448,29 +444,26 @@ int NetworkGraph::realizeGraph() {
     return ML_ERROR_INVALID_PARAMETER;
   }
 
-  num_nodes = graph.size();
-  node_list = graph.getNodes();
-
-  for (unsigned int i = 0; i < num_nodes; ++i) {
-    auto const &lnode = LNODE(node_list[i]);
+  /**
+   * invariant: the new realized nodes are added to the end,
+   * otherwise this iteration becomes invalid. So, every iteration must be fresh
+   * iterator as vector resize invalidates all the iterators.
+   */
+  for (unsigned int i = 0; i < graph.size(); ++i) {
+    auto const &lnode = LNODE(*(cbegin() + i));
     if (lnode->getType() != OutputLayer::type &&
         lnode->getType() != SplitLayer::type) {
       status = realizeMultiOutputType(lnode);
       NN_RETURN_STATUS();
     }
   }
-
-  num_nodes = graph.size();
-  node_list = graph.getNodes();
-
   /// @todo add check that input_layers <-> output_layers does match.
 
   return status;
 }
 
 void NetworkGraph::setBatchSize(unsigned int batch_size) {
-  auto const &node_list = graph.getNodes();
-  for (auto const &node : node_list) {
+  for (auto const &node : graph) {
     LNODE(node)->getObject()->setBatch(batch_size);
   }
 }
@@ -512,13 +505,10 @@ NetworkGraph::getUnsortedLayers(const std::string &input_layer,
   /// Further, this function must be removed. There should be rather
   /// getAllNames and getLayerByName instead of getUnsortedLayers.
 
-  auto const &unsortedNodes = graph.getNodes();
-
   /** count layers after output layer */
   unsigned int num_layers_remove_end = 0;
   if (!output_layer.empty()) {
-    for (auto iter = unsortedNodes.rbegin(); iter != unsortedNodes.rend();
-         iter++) {
+    for (auto iter = graph.crbegin(); iter != graph.crend(); iter++) {
       if ((*iter)->getName() != output_layer)
         num_layers_remove_end++;
       else
@@ -532,8 +522,8 @@ NetworkGraph::getUnsortedLayers(const std::string &input_layer,
   /** count layers before input layer */
   unsigned int num_layers_remove_start = 0;
   if (!input_layer.empty()) {
-    for (auto iter = unsortedNodes.begin();
-         iter != unsortedNodes.end() - num_layers_remove_end; iter++) {
+    for (auto iter = graph.cbegin();
+         iter != graph.cend() - num_layers_remove_end; iter++) {
       if ((*iter)->getName() != input_layer)
         num_layers_remove_start++;
       else
@@ -543,21 +533,15 @@ NetworkGraph::getUnsortedLayers(const std::string &input_layer,
 
   /** copy the graph and return */
   std::vector<std::shared_ptr<LayerNode>> ret;
-  std::transform(unsortedNodes.begin() + num_layers_remove_start,
-                 unsortedNodes.end() - num_layers_remove_end,
-                 std::back_inserter(ret),
+  std::transform(graph.cbegin() + num_layers_remove_start,
+                 graph.cend() - num_layers_remove_end, std::back_inserter(ret),
                  [](auto const &elem) { return LNODE(elem); });
 
   return ret;
 }
 
 std::vector<std::shared_ptr<LayerNode>> NetworkGraph::getLayerNodes() const {
-  auto nodes = graph.getNodes();
-  std::vector<std::shared_ptr<LayerNode>> ret;
-  std::transform(nodes.begin(), nodes.end(), std::back_inserter(ret),
-                 [](auto const &elem) { return LNODE(elem); });
-
-  return ret;
+  return std::vector<std::shared_ptr<LayerNode>>(cbegin(), cend());
 }
 
 void NetworkGraph::extendGraph(std::vector<std::shared_ptr<LayerNode>> ex_graph,