[graph] Bug fix for addLossLayer

Removing the last activation is guaranteed to be the last
layer in the Sorted list, but not in the adj graph.
This patch fixes the way the last node is removed from the adj.

Also add some cleanup.

**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/Applications/Custom/LayerPlugin/layer_plugin_test.cpp b/Applications/Custom/LayerPlugin/layer_plugin_test.cpp
index 0b21b3b..745f47e 100644 (file)
--- a/Applications/Custom/LayerPlugin/layer_plugin_test.cpp
+++ b/Applications/Custom/LayerPlugin/layer_plugin_test.cpp
@@ -19,7 +19,7 @@
 
 #include <app_context.h>
 #include <layer.h>
-#include <layer_node.h>
+#include <layer_internal.h>
 
 const char *NNTRAINER_PATH = std::getenv("NNTRAINER_PATH");
 

diff --git a/Applications/Embedding/jni/main.cpp b/Applications/Embedding/jni/main.cpp
index c1e12e5..c2fcd50 100644 (file)
--- a/Applications/Embedding/jni/main.cpp
+++ b/Applications/Embedding/jni/main.cpp
@@ -19,13 +19,13 @@
 #include <iostream>
 #include <sstream>
 #include <stdlib.h>
-#include <tensor.h>
 #include <time.h>
 
 #include <dataset.h>
-#include <layer_node.h>
+#include <layer_internal.h>
 #include <ml-api-common.h>
 #include <neuralnet.h>
+#include <tensor.h>
 
 std::string data_file;
 

diff --git a/Applications/SimpleShot/test/simpleshot_centering_test.cpp b/Applications/SimpleShot/test/simpleshot_centering_test.cpp
index 2205f25..c074c63 100644 (file)
--- a/Applications/SimpleShot/test/simpleshot_centering_test.cpp
+++ b/Applications/SimpleShot/test/simpleshot_centering_test.cpp
@@ -16,7 +16,7 @@
 #include <memory>
 
 #include <app_context.h>
-#include <layer_node.h>
+#include <layer_internal.h>
 #include <manager.h>
 #include <nntrainer_test_util.h>
 

diff --git a/Applications/SimpleShot/test/simpleshot_l2norm_test.cpp b/Applications/SimpleShot/test/simpleshot_l2norm_test.cpp
index 6d42c5e..6c0019f 100644 (file)
--- a/Applications/SimpleShot/test/simpleshot_l2norm_test.cpp
+++ b/Applications/SimpleShot/test/simpleshot_l2norm_test.cpp
@@ -15,7 +15,7 @@
 #include <memory>
 
 #include <app_context.h>
-#include <layer_node.h>
+#include <layer_internal.h>
 #include <manager.h>
 #include <nntrainer_test_util.h>
 

diff --git a/nntrainer/graph/network_graph.cpp b/nntrainer/graph/network_graph.cpp
index 9dac4e1..0e76aca 100644 (file)
--- a/nntrainer/graph/network_graph.cpp
+++ b/nntrainer/graph/network_graph.cpp
@@ -404,9 +404,8 @@ int NetworkGraph::addLossLayer(const LossType loss_type) {
       return ML_ERROR_NOT_SUPPORTED;
     }
 
-    /// TODO: the last entry in adj and sorted are not same
-    adj.pop_back();
     Sorted.pop_back();
+    adj.erase(adj.begin() + last_node->getIndex());
 
     switch (last_node->getObject()->getActivationType()) {
     case ActivationType::ACT_SIGMOID:
@@ -421,11 +420,13 @@ int NetworkGraph::addLossLayer(const LossType loss_type) {
     }
   }
 
-  /// @note this assumes that loss layer only supports single input
   last_node = Sorted.back();
 
-  /// TODO: need to find all the nodes whose entry matches with node remove and
-  /// update them all
+  /**
+   * Remove all the connections for the current lasy layer as it will now only
+   * connect with the new loss layer to be added.
+   * @note this assumes that loss layer only supports single input
+   */
   if (updated_loss_type == LossType::LOSS_ENTROPY_SIGMOID ||
       updated_loss_type == LossType::LOSS_ENTROPY_SOFTMAX)
     adj[last_node->getIndex()].resize(1);

diff --git a/nntrainer/layers/layer.cpp b/nntrainer/layers/layer.cpp
index 0c6a309..9777dbd 100644 (file)
--- a/nntrainer/layers/layer.cpp
+++ b/nntrainer/layers/layer.cpp
@@ -24,6 +24,7 @@
 #include <sstream>
 
 #include <layer_internal.h>
+#include <layer_node.h>
 #include <nntrainer_error.h>
 #include <nntrainer_log.h>
 #include <parse_util.h>
@@ -415,4 +416,8 @@ void Layer::print(std::ostream &out, unsigned int flags) {
   }
 };
 
+std::shared_ptr<Layer> getLayerDevel(std::shared_ptr<ml::train::Layer> l) {
+  return std::static_pointer_cast<LayerNode>(l)->getObject();
+}
+
 } /* namespace nntrainer */

diff --git a/nntrainer/layers/layer_internal.h b/nntrainer/layers/layer_internal.h
index ce7a24e..e88e6f7 100644 (file)
--- a/nntrainer/layers/layer_internal.h
+++ b/nntrainer/layers/layer_internal.h
@@ -759,6 +759,14 @@ std::unique_ptr<Layer> createLayer(const std::vector<std::string> &props = {}) {
   return ptr;
 }
 
+/**
+ * @brief   Get Layer devel from ml::train::Layer
+ *
+ * @param   l Layer object
+ * @return  Layer devel object
+ */
+std::shared_ptr<Layer> getLayerDevel(std::shared_ptr<ml::train::Layer> l);
+
 } // namespace nntrainer
 
 #endif /* __cplusplus */

diff --git a/nntrainer/layers/layer_node.cpp b/nntrainer/layers/layer_node.cpp
index 1b85bc9..20b8cb4 100644 (file)
--- a/nntrainer/layers/layer_node.cpp
+++ b/nntrainer/layers/layer_node.cpp
@@ -12,14 +12,4 @@
 
 #include <layer_node.h>
 
-namespace nntrainer {
-
-std::shared_ptr<Layer> getLayerDevel(std::shared_ptr<ml::train::Layer> l) {
-  std::shared_ptr<LayerNode> lnode = std::static_pointer_cast<LayerNode>(l);
-
-  std::shared_ptr<Layer> &layer = lnode->getObject();
-
-  return layer;
-}
-
-}; // namespace nntrainer
+namespace nntrainer {}; // namespace nntrainer

diff --git a/nntrainer/layers/layer_node.h b/nntrainer/layers/layer_node.h
index f62fe61..8448a64 100644 (file)
--- a/nntrainer/layers/layer_node.h
+++ b/nntrainer/layers/layer_node.h
@@ -131,13 +131,5 @@ private:
     activation_type; /**< activation applied to the output of this node */
 };
 
-/**
- * @brief   Get Layer devel from ml::train::Layer
- *
- * @param   l Layer object
- * @return  Layer devel object
- */
-std::shared_ptr<Layer> getLayerDevel(std::shared_ptr<ml::train::Layer> l);
-
 } // namespace nntrainer
 #endif // __LAYER_NODE_H__

diff --git a/test/unittest/compiler/unittest_interpreter.cpp b/test/unittest/compiler/unittest_interpreter.cpp
index aae3323..95129e1 100644 (file)
--- a/test/unittest/compiler/unittest_interpreter.cpp
+++ b/test/unittest/compiler/unittest_interpreter.cpp
@@ -34,6 +34,7 @@ makeGraph(const std::vector<LayerReprentation> &layer_reps) {
   auto graph = std::make_shared<nntrainer::GraphRepresentation>();
 
   for (const auto &layer_representation : layer_reps) {
+    /// @todo Use unique_ptr here
     std::shared_ptr<nntrainer::Layer> nntr_layer =
       ac.createObject<nntrainer::Layer>(layer_representation.first,
                                         layer_representation.second);

diff --git a/test/unittest/unittest_nntrainer_models.cpp b/test/unittest/unittest_nntrainer_models.cpp
index ec95caf..3f7ce60 100644 (file)
--- a/test/unittest/unittest_nntrainer_models.cpp
+++ b/test/unittest/unittest_nntrainer_models.cpp
@@ -385,7 +385,7 @@ GraphWatcher::GraphWatcher(const std::string &config, const bool opt) :
   std::vector<NodeType> graph = model_graph.getSorted();
 
   for (auto it = graph.begin(); it != graph.end() - 1; ++it) {
-    nodes.push_back(NodeWatcher((*it)));
+    nodes.push_back(NodeWatcher(*it));
   }
 
   loss_node = NodeWatcher(graph.back());