From 66ed1e46b26fa72c00261aa72f6d0900d0219bd2 Mon Sep 17 00:00:00 2001
From: Valentin Seitz <valentin.seitz@bsc.es>
Date: Wed, 24 Jul 2024 12:20:15 +0200
Subject: [PATCH 1/5] Refactored and made the delegator a bit easier

---
 src/delegator.cpp | 151 ++++++++++++++++++++++------------------------
 src/delegator.hpp |  15 +++--
 2 files changed, 80 insertions(+), 86 deletions(-)

diff --git a/src/delegator.cpp b/src/delegator.cpp
index 60f68f5..547a317 100644
--- a/src/delegator.cpp
+++ b/src/delegator.cpp
@@ -1,6 +1,7 @@
 #include <delegator.hpp>
 #include <iostream>
 #include <memory>
+#include <optional>
 #include <sstream>
 #include <strategies.hpp>
 #include <string>
@@ -19,8 +20,18 @@
 #include "backends/extrae/extrae_type_stack.hpp"
 #endif
 
-void Delegator::InitDetectionBackend(const std::string &backend) {
-  pn_annotation_strategy_ = std::make_unique<DetectionStrategy>();
+std::optional<NestingMode> getNestingMode(const std::string &nesting_mode) {
+  // detect nesting mode specified by the user
+  if (stringsAreCaseInsensitiveEqual(nesting_mode, "?")) {
+    return NestingMode::Detection;
+  } else if (stringsAreCaseInsensitiveEqual(nesting_mode, "ProperlyNested")) {
+    return NestingMode::ProperlyNested;
+  } else if (stringsAreCaseInsensitiveEqual(nesting_mode,
+                                            "NotProperlyNested")) {
+    return NestingMode::NotProperlyNested;
+  } else {
+    return std::nullopt;
+  }
 }
 
 void Delegator::InitNonProperlyNestedBackends(const std::string &backend) {
@@ -37,12 +48,13 @@ void Delegator::InitNonProperlyNestedBackends(const std::string &backend) {
   else {
     npn_annotation_strategy_ = std::make_unique<NPNDefault>();
 
-    std::cout << "Could not find a matching NotProperlyNested backend with name"
-              << backend << " now selecting the Default" << std::endl;
+    std::cout
+        << "Could not find a matching NotProperlyNested backend with name "
+        << backend << " now selecting the Default" << std::endl;
   }
 }
 
-void Delegator::InitProperlyNestedBackends(const std::string &backend) {
+void Delegator::TryInitProperlyNestedBackend(const std::string &backend) {
   if (false) {
     // a bit ugly, but worky
   }
@@ -58,28 +70,28 @@ void Delegator::InitProperlyNestedBackends(const std::string &backend) {
 #endif
 }
 
-void Delegator::Init(std::string_view nesting_mode,
+void Delegator::Init(std::string_view nesting_mode_in,
                      std::string_view backend_in) {
   mpi_helper_ = std::make_unique<MPIHelper>();
+
   std::string backend = std::string(backend_in);
 
   if (mpi_helper_->IsRankNumber(0)) {
-    std::cout << "NESMIK init called with: " << nesting_mode
-              << " and Backend: " << backend << std::endl;
+    std::cout << "neSmiK init called with program nesting: " << nesting_mode_in
+              << " and backend: " << backend << std::endl;
   }
 
-  // detect nesting mode specified by the user
-  if (nesting_mode.compare("?") == 0) {
-    nesting_mode_ = NestingMode::Detection;
-  } else if (nesting_mode.compare("ProperlyNested") == 0) {
-    nesting_mode_ = NestingMode::ProperlyNested;
-  } else if (nesting_mode.compare("NotProperlyNested") == 0) {
-    nesting_mode_ = NestingMode::NotProperlyNested;
+  // now determine nesting mode
+  auto nesting_mode = getNestingMode(std::string(nesting_mode_in));
+  if (!nesting_mode.has_value()) {
+    std::cout << "Something went wrong when selecting the nesting mode, "
+                 "please make sure to spell it correctly"
+              << std::endl;
+    exit(1);
   }
-
   if (mpi_helper_->IsRankNumber(0)) {
     // Some debug output
-    switch (nesting_mode_) {
+    switch (nesting_mode.value()) {
       case NestingMode::Detection:
         std::cout << "nesting mode set to: "
                   << "Detection (?)" << std::endl;
@@ -92,11 +104,6 @@ void Delegator::Init(std::string_view nesting_mode,
         std::cout << "nesting mode set to: "
                   << "ProperlyNested" << std::endl;
         break;
-      case NestingMode::ProperlyNestedButOtherBackend:
-        std::cout << "Something went wrong when selecting the nesting mode"
-                  << std::endl;
-        exit(1);
-        break;
     }
   }
 
@@ -117,41 +124,48 @@ void Delegator::Init(std::string_view nesting_mode,
     }
   }
 
-  // next choose backend
-  switch (nesting_mode_) {
-    case NestingMode::ProperlyNestedButOtherBackend:
-      break;
+  // next do backend selection:
+
+  switch (nesting_mode.value()) {
     case NestingMode::Detection:
-      InitDetectionBackend(backend);
+      pn_annotation_strategy_ = std::make_unique<DetectionStrategy>();
+      backend_selection_ = BackendSelection::ProperlyNested;
       break;
     case NestingMode::ProperlyNested:
-      InitProperlyNestedBackends(backend);
-      // we preferably use a Properly nested backend,
+      TryInitProperlyNestedBackend(backend);
+      //  we preferably use a Properly nested backend,
       //  but if this is not possible, we will select the not properly nested
       //  one
+      if (pn_annotation_strategy_ != nullptr) {
+        backend_selection_ = BackendSelection::ProperlyNested;
+        break;  // we break here if we already inited the backend
+      }
       [[fallthrough]];
     case NestingMode::NotProperlyNested:
-      if (pn_annotation_strategy_ == nullptr && mpi_helper_->IsRankNumber(0)) {
+      // two cases lead us here: either we previously tried to init a properly
+      // nested backend, which failed. if this failed, we inform the user of
+      // this
+      if (nesting_mode.value() == NestingMode::ProperlyNested &&
+          pn_annotation_strategy_ == nullptr && mpi_helper_->IsRankNumber(0)) {
         std::cout
             << "Could not find a matching ProperlyNested backend with name "
             << backend << std::endl;
         std::cout << "Now looking for a NotProperlyNested backend, as they "
                      "support ProperlyNested ones"
                   << std::endl;
-        nesting_mode_ = NestingMode::ProperlyNestedButOtherBackend;
       }
-
       InitNonProperlyNestedBackends(backend);
+      if (npn_annotation_strategy_ != nullptr) {
+        backend_selection_ = BackendSelection::NotProperlyNested;
+      }
       break;
   }
 
-  switch (nesting_mode_) {
-    case NestingMode::NotProperlyNested:
-    case NestingMode::ProperlyNestedButOtherBackend:
+  switch (backend_selection_) {
+    case BackendSelection::NotProperlyNested:
       return npn_annotation_strategy_->Init();
       break;
-    case NestingMode::Detection:
-    case NestingMode::ProperlyNested:
+    case BackendSelection::ProperlyNested:
       return pn_annotation_strategy_->Init();
       break;
   }
@@ -159,28 +173,22 @@ void Delegator::Init(std::string_view nesting_mode,
 
 // should be thread safe to call?
 void Delegator::RegionStart(std::string_view name) {
-  switch (nesting_mode_) {
-    case NestingMode::NotProperlyNested:
-      return npn_annotation_strategy_->RegionStart({name});
-    case NestingMode::ProperlyNestedButOtherBackend:
-      name_stack_.push(std::string(name));
+  name_stack_.push(std::string(name));
+  switch (backend_selection_) {
+    case BackendSelection::NotProperlyNested:
       return npn_annotation_strategy_->RegionStart({name});
-    case NestingMode::Detection:
-    case NestingMode::ProperlyNested:
-      // First push to stack
-      name_stack_.push(std::string(name));
+    case BackendSelection::ProperlyNested:
       return pn_annotation_strategy_->RegionStart({name, name_stack_});
       break;
   }
 }
+
 void Delegator::RegionStop(std::string_view name) {
-  switch (nesting_mode_) {
-    case NestingMode::NotProperlyNested:
-    case NestingMode::ProperlyNestedButOtherBackend:
+  switch (backend_selection_) {
+    case BackendSelection::NotProperlyNested:
       return npn_annotation_strategy_->RegionStop({name});
       break;
-    case NestingMode::Detection:
-    case NestingMode::ProperlyNested:
+    case BackendSelection::ProperlyNested:
       pn_annotation_strategy_->RegionStopLast({name, name_stack_});
       if (!name_stack_.empty()) {
         name_stack_.pop();
@@ -190,41 +198,28 @@ void Delegator::RegionStop(std::string_view name) {
 };
 
 void Delegator::RegionStopLast() {
-  switch (nesting_mode_) {
-    case NestingMode::NotProperlyNested:
-      std::cout << "Calling region_stop_last() is not supported in "
-                   "NotProperlyNested Codes"
-                << std::endl;
+  const std::string stack_empty = "NESMIK: STACK EMPTPY";
+  const std::string &last_region_name =
+      name_stack_.empty() ? stack_empty : name_stack_.top();
+  switch (backend_selection_) {
+    case BackendSelection::ProperlyNested:
+      pn_annotation_strategy_->RegionStopLast({last_region_name, name_stack_});
       break;
-    case NestingMode::Detection:
-    case NestingMode::ProperlyNested:
-    case NestingMode::ProperlyNestedButOtherBackend:
-      const std::string stack_empty = "NESMIK: STACK EMPTPY";
-      const std::string &last_region_name =
-          name_stack_.empty() ? stack_empty : name_stack_.top();
-
-      if (nesting_mode_ == NestingMode::Detection ||
-          nesting_mode_ == NestingMode::ProperlyNested) {
-        pn_annotation_strategy_->RegionStopLast(
-            {last_region_name, name_stack_});
-      } else if (nesting_mode_ == NestingMode::ProperlyNestedButOtherBackend) {
-        npn_annotation_strategy_->RegionStop({last_region_name});
-      }
-      if (!name_stack_.empty()) {
-        name_stack_.pop();
-      }
+    case BackendSelection::NotProperlyNested:
+      npn_annotation_strategy_->RegionStop({last_region_name});
       break;
   }
+  if (!name_stack_.empty()) {
+    name_stack_.pop();
+  }
 }
 
 void Delegator::Finalize() {
-  switch (nesting_mode_) {
-    case NestingMode::NotProperlyNested:
-    case NestingMode::ProperlyNestedButOtherBackend:
+  switch (backend_selection_) {
+    case BackendSelection::NotProperlyNested:
       return npn_annotation_strategy_->Finalize();
       break;
-    case NestingMode::Detection:
-    case NestingMode::ProperlyNested:
+    case BackendSelection::ProperlyNested:
       return pn_annotation_strategy_->Finalize();
       break;
   }
diff --git a/src/delegator.hpp b/src/delegator.hpp
index 021ea81..deb18e6 100644
--- a/src/delegator.hpp
+++ b/src/delegator.hpp
@@ -8,13 +8,13 @@
 #include <vector>
 
 enum class NestingMode {
-  Detection,          // When selecting the very special Detection backend
-  ProperlyNested,     // Application is properly nested
-  NotProperlyNested,  // Application annotation is not properly nested
-  ProperlyNestedButOtherBackend  // Application is properly nested, but were
-                                 // using a not properly nested backend
+  Detection,         // When selecting the very special Detection backend
+  ProperlyNested,    // Application is properly nested
+  NotProperlyNested  // Application annotation is not properly nested
 };
 
+enum class BackendSelection { ProperlyNested, NotProperlyNested };
+
 class Delegator {
  private:
   inline static std::unique_ptr<ProperlyNestedAnnotationStrategy>
@@ -24,7 +24,7 @@ class Delegator {
 
   inline static thread_local std::stack<std::string> name_stack_;
 
-  inline static NestingMode nesting_mode_ = NestingMode::Detection;
+  inline static BackendSelection backend_selection_;
 
   inline static std::unique_ptr<MPIHelper> mpi_helper_;
 
@@ -36,8 +36,7 @@ class Delegator {
   static void Finalize();
 
  private:
-  static void InitDetectionBackend(const std::string &backend);
   static void InitNonProperlyNestedBackends(const std::string &backend);
-  static void InitProperlyNestedBackends(const std::string &backend);
+  static void TryInitProperlyNestedBackend(const std::string &backend);
 };
 #endif  // NESMIK_DELEGATOR_HPP
-- 
GitLab


From 1c12c977f50d99cfd3230ac51a06b8a789cb0f93 Mon Sep 17 00:00:00 2001
From: Valentin Seitz <valentin.seitz@bsc.es>
Date: Wed, 24 Jul 2024 14:20:28 +0200
Subject: [PATCH 2/5] Fix mixed calls

---
 src/delegator.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/delegator.cpp b/src/delegator.cpp
index 547a317..b0f42b5 100644
--- a/src/delegator.cpp
+++ b/src/delegator.cpp
@@ -190,11 +190,11 @@ void Delegator::RegionStop(std::string_view name) {
       break;
     case BackendSelection::ProperlyNested:
       pn_annotation_strategy_->RegionStopLast({name, name_stack_});
-      if (!name_stack_.empty()) {
-        name_stack_.pop();
-      }
       break;
   }
+  if (!name_stack_.empty()) {
+    name_stack_.pop();
+  }
 };
 
 void Delegator::RegionStopLast() {
-- 
GitLab


From 417c696b60c41ad51a31ffe664c8ec3f6ababc0e Mon Sep 17 00:00:00 2001
From: Valentin Seitz <valentin.seitz@bsc.es>
Date: Wed, 24 Jul 2024 14:43:16 +0200
Subject: [PATCH 3/5] remove return, which stopped stack from getting popped

---
 src/delegator.cpp                | 13 ++++++++++---
 tests/cpp/TestRegionStopLast.cpp |  2 +-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/delegator.cpp b/src/delegator.cpp
index b0f42b5..4e56797 100644
--- a/src/delegator.cpp
+++ b/src/delegator.cpp
@@ -174,11 +174,13 @@ void Delegator::Init(std::string_view nesting_mode_in,
 // should be thread safe to call?
 void Delegator::RegionStart(std::string_view name) {
   name_stack_.push(std::string(name));
+
   switch (backend_selection_) {
     case BackendSelection::NotProperlyNested:
-      return npn_annotation_strategy_->RegionStart({name});
+      npn_annotation_strategy_->RegionStart({name});
+      break;
     case BackendSelection::ProperlyNested:
-      return pn_annotation_strategy_->RegionStart({name, name_stack_});
+      pn_annotation_strategy_->RegionStart({name, name_stack_});
       break;
   }
 }
@@ -186,13 +188,17 @@ void Delegator::RegionStart(std::string_view name) {
 void Delegator::RegionStop(std::string_view name) {
   switch (backend_selection_) {
     case BackendSelection::NotProperlyNested:
-      return npn_annotation_strategy_->RegionStop({name});
+      npn_annotation_strategy_->RegionStop({name});
       break;
     case BackendSelection::ProperlyNested:
       pn_annotation_strategy_->RegionStopLast({name, name_stack_});
       break;
   }
   if (!name_stack_.empty()) {
+    if (name.compare(name_stack_.top()) != 0) {
+      std::cout << "neSmiK region_close() expected " << name_stack_.top()
+                << "but got " << name << std::endl;
+    }
     name_stack_.pop();
   }
 };
@@ -209,6 +215,7 @@ void Delegator::RegionStopLast() {
       npn_annotation_strategy_->RegionStop({last_region_name});
       break;
   }
+
   if (!name_stack_.empty()) {
     name_stack_.pop();
   }
diff --git a/tests/cpp/TestRegionStopLast.cpp b/tests/cpp/TestRegionStopLast.cpp
index 85c3e49..6bc07cb 100644
--- a/tests/cpp/TestRegionStopLast.cpp
+++ b/tests/cpp/TestRegionStopLast.cpp
@@ -4,7 +4,7 @@ int main() {
   nesmik::init("ProperlyNested", "Default");
   nesmik::region_start("Test0-1");
   nesmik::region_start("Test1-1");
-  nesmik::region_stop_last();
+  nesmik::region_stop("Test1-1");
   nesmik::region_start("Test1-2");
   nesmik::region_stop_last();
   nesmik::region_stop_last();
-- 
GitLab


From 73be96b978144b01612c870df0c1f6889db18e00 Mon Sep 17 00:00:00 2001
From: VALENTIN SEITZ <valentin.seitz@bsc.es>
Date: Wed, 24 Jul 2024 15:54:56 +0200
Subject: [PATCH 4/5] Apply 1 suggestion(s) to 1 file(s)

Co-authored-by: JOAN VINYALS YLLA CATALA <joan.vinyals@bsc.es>
---
 src/delegator.cpp | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/delegator.cpp b/src/delegator.cpp
index 4e56797..9a7861a 100644
--- a/src/delegator.cpp
+++ b/src/delegator.cpp
@@ -140,13 +140,10 @@ void Delegator::Init(std::string_view nesting_mode_in,
         backend_selection_ = BackendSelection::ProperlyNested;
         break;  // we break here if we already inited the backend
       }
-      [[fallthrough]];
-    case NestingMode::NotProperlyNested:
-      // two cases lead us here: either we previously tried to init a properly
-      // nested backend, which failed. if this failed, we inform the user of
-      // this
-      if (nesting_mode.value() == NestingMode::ProperlyNested &&
-          pn_annotation_strategy_ == nullptr && mpi_helper_->IsRankNumber(0)) {
+
+      // we inform the user that ProperlyNested initialization failed, and 
+      // fallthrough
+      if (mpi_helper_->IsRankNumber(0)) {
         std::cout
             << "Could not find a matching ProperlyNested backend with name "
             << backend << std::endl;
@@ -154,6 +151,11 @@ void Delegator::Init(std::string_view nesting_mode_in,
                      "support ProperlyNested ones"
                   << std::endl;
       }
+
+      [[fallthrough]];
+    case NestingMode::NotProperlyNested:
+      // two cases lead us here: either we previously tried to init a properly
+      // nested backend, which failed, or we are in not properly nested mode.
       InitNonProperlyNestedBackends(backend);
       if (npn_annotation_strategy_ != nullptr) {
         backend_selection_ = BackendSelection::NotProperlyNested;
-- 
GitLab


From c3436a395c715f3ccb0fab1134b6b80a6ec3b916 Mon Sep 17 00:00:00 2001
From: Valentin Seitz <valentin.seitz@bsc.es>
Date: Wed, 24 Jul 2024 16:00:30 +0200
Subject: [PATCH 5/5] added clang-format

---
 src/delegator.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/delegator.cpp b/src/delegator.cpp
index 9a7861a..51ff083 100644
--- a/src/delegator.cpp
+++ b/src/delegator.cpp
@@ -141,7 +141,7 @@ void Delegator::Init(std::string_view nesting_mode_in,
         break;  // we break here if we already inited the backend
       }
 
-      // we inform the user that ProperlyNested initialization failed, and 
+      // we inform the user that ProperlyNested initialization failed, and
       // fallthrough
       if (mpi_helper_->IsRankNumber(0)) {
         std::cout
-- 
GitLab