Replace spdlog output with exception messages (#11)

Isaac Poulton · web-flow · commit 485e5b9dae46 · 2019-04-24T13:07:38.000+09:00
* Replace spdlog output with exception messages

* Move spdlog into example directory

* Allow disabling of example and test executables
diff --git a/.gitmodules b/.gitmodules
@@ -1,5 +1,5 @@
 [submodule "lib/spdlog"]
-	path = lib/spdlog
+	path = example/lib/spdlog
 	url = git@github.com:gabime/spdlog.git
 [submodule "lib/msgpack-c"]
 	path = example/lib/msgpack-c
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -24,7 +24,6 @@ list(APPEND CPPCHECK_ARGS
     --suppressions-list=${CMAKE_CURRENT_LIST_DIR}/CppCheckSuppressions.txt 
     -I ${CMAKE_CURRENT_LIST_DIR}/src 
     -I ${CMAKE_CURRENT_LIST_DIR}/include 
-    -I ${CMAKE_CURRENT_LIST_DIR}/lib/spdlog/include 
     -I ${CMAKE_CURRENT_LIST_DIR}/example 
     ${CMAKE_CURRENT_LIST_DIR}/src
     ${CMAKE_CURRENT_LIST_DIR}/example
@@ -42,8 +41,6 @@ find_package(Torch REQUIRED)
 if (TORCH_CXX_FLAGS)
     set(CMAKE_CXX_FLAGS ${TORCH_CXX_FLAGS})
 endif()
-## Spdlog
-add_subdirectory(lib/spdlog)
 
 # Define targets
 add_library(cpprl STATIC "")
@@ -64,18 +61,25 @@ endif(MSVC)
 set(CPPRL_INCLUDE_DIRS
     include
     src
-    lib/spdlog/include
     ${TORCH_INCLUDE_DIRS}
 )
 target_include_directories(cpprl PRIVATE ${CPPRL_INCLUDE_DIRS})
-target_include_directories(cpprl_tests PRIVATE ${CPPRL_INCLUDE_DIRS})
+if (CPPRL_BUILD_TESTS)
+    target_include_directories(cpprl_tests PRIVATE ${CPPRL_INCLUDE_DIRS})
+endif(CPPRL_BUILD_TESTS)
 
 # Linking
 target_link_libraries(cpprl torch ${TORCH_LIBRARIES})
-target_link_libraries(cpprl_tests torch ${TORCH_LIBRARIES})
+target_link_libraries(cpprl torch ${TORCH_LIBRARIES})
+if (CPPRL_BUILD_TESTS)
+    target_link_libraries(cpprl_tests torch ${TORCH_LIBRARIES})
+endif(CPPRL_BUILD_TESTS)
 
 # Example
-add_subdirectory(example)
+option(CPPRL_BUILD_EXAMPLE "Whether or not to build the CppRl Gym example" ON)
+if (CPPRL_BUILD_EXAMPLE)
+    add_subdirectory(example)
+endif(CPPRL_BUILD_EXAMPLE)
 
 # Recurse into source tree
 add_subdirectory(src)
diff --git a/example/CMakeLists.txt b/example/CMakeLists.txt
@@ -3,8 +3,11 @@ add_executable(gym_client gym_client.cpp communicator.cpp)
 set(LIB_DIR ${CMAKE_CURRENT_LIST_DIR}/lib)
 set(CPPZMQ_DIR ${LIB_DIR}/cppzmq)
 set(MSGPACK_DIR ${LIB_DIR}/msgpack-c)
+set(SPDLOG_DIR ${LIB_DIR}/spdlog)
 set(ZMQ_DIR ${LIB_DIR}/libzmq)
 
+# Spdlog
+add_subdirectory(${SPDLOG_DIR})
 # ZMQ
 option(ZMQ_BUILD_TESTS "" OFF)
 add_subdirectory(${ZMQ_DIR})
@@ -16,6 +19,7 @@ target_include_directories(gym_client
     ../lib/spdlog/include
     ${CPPZMQ_DIR}
     ${MSGPACK_DIR}/include
+    ${SPDLOG_DIR}/include
     ${ZMQ_DIR}/include
 )
 
diff --git a/example/lib/spdlog b/example/lib/spdlog
diff --git a/src/algorithms/ppo.cpp b/src/algorithms/ppo.cpp
@@ -1,7 +1,6 @@
 #include <chrono>
 #include <memory>
 
-#include <spdlog/spdlog.h>
 #include <torch/torch.h>
 
 #include "cpprl/algorithms/ppo.h"
diff --git a/src/distributions/bernoulli.cpp b/src/distributions/bernoulli.cpp
@@ -1,6 +1,5 @@
 #include <ATen/core/Reduction.h>
 #include <c10/util/ArrayRef.h>
-#include <spdlog/spdlog.h>
 #include <torch/torch.h>
 
 #include "cpprl/distributions/bernoulli.h"
@@ -13,15 +12,14 @@ Bernoulli::Bernoulli(const torch::Tensor *probs,
 {
     if ((probs == nullptr) == (logits == nullptr))
     {
-        spdlog::error("Either probs or logits is required, but not both");
-        throw std::exception();
+        throw std::runtime_error("Either probs or logits is required, but not both");
     }
 
     if (probs != nullptr)
     {
         if (probs->dim() < 1)
         {
-            throw std::exception();
+            throw std::runtime_error("Probabilities tensor must have at least one dimension");
         }
         this->probs = *probs;
         // 1.21e-7 is used as the epsilon to match PyTorch's Python results as closely
@@ -33,7 +31,7 @@ Bernoulli::Bernoulli(const torch::Tensor *probs,
     {
         if (logits->dim() < 1)
         {
-            throw std::exception();
+            throw std::runtime_error("Logits tensor must have at least one dimension");
         }
         this->logits = *logits;
         this->probs = torch::sigmoid(*logits);
diff --git a/src/distributions/categorical.cpp b/src/distributions/categorical.cpp
@@ -1,5 +1,4 @@
 #include <c10/util/ArrayRef.h>
-#include <spdlog/spdlog.h>
 #include <torch/torch.h>
 
 #include "cpprl/distributions/categorical.h"
@@ -12,15 +11,14 @@ Categorical::Categorical(const torch::Tensor *probs,
 {
     if ((probs == nullptr) == (logits == nullptr))
     {
-        spdlog::error("Either probs or logits is required, but not both");
-        throw std::exception();
+        throw std::runtime_error("Either probs or logits is required, but not both");
     }
 
     if (probs != nullptr)
     {
         if (probs->dim() < 1)
         {
-            throw std::exception();
+            throw std::runtime_error("Probabilities tensor must have at least one dimension");
         }
         this->probs = *probs / probs->sum(-1, true);
         // 1.21e-7 is used as the epsilon to match PyTorch's Python results as closely
@@ -32,7 +30,7 @@ Categorical::Categorical(const torch::Tensor *probs,
     {
         if (logits->dim() < 1)
         {
-            throw std::exception();
+            throw std::runtime_error("Logits tensor must have at least one dimension");
         }
         this->logits = *logits - logits->logsumexp(-1, true);
         this->probs = torch::softmax(this->logits, -1);
diff --git a/src/generators/feed_forward_generator.cpp b/src/generators/feed_forward_generator.cpp
@@ -1,7 +1,6 @@
 #include <algorithm>
 #include <vector>
 
-#include <spdlog/spdlog.h>
 #include <torch/torch.h>
 
 #include "cpprl/generators/feed_forward_generator.h"
@@ -44,10 +43,7 @@ MiniBatch FeedForwardGenerator::next()
 {
     if (index >= indices.size(0))
     {
-        spdlog::error("No minibatches left in generator. Index {}, minibatch "
-                      "count: {}.",
-                      index, indices.size(0));
-        throw std::exception();
+        throw std::runtime_error("No minibatches left in generator.");
     }
 
     MiniBatch mini_batch;
diff --git a/src/generators/recurrent_generator.cpp b/src/generators/recurrent_generator.cpp
@@ -1,7 +1,6 @@
 #include <algorithm>
 #include <vector>
 
-#include <spdlog/spdlog.h>
 #include <torch/torch.h>
 
 #include "cpprl/generators/recurrent_generator.h"
@@ -49,10 +48,7 @@ MiniBatch RecurrentGenerator::next()
 {
     if (index >= indices.size(0))
     {
-        spdlog::error("No minibatches left in generator. Index {}, minibatch "
-                      "count: {}.",
-                      index, indices.size(0));
-        throw std::exception();
+        throw std::runtime_error("No minibatches left in generator.");
     }
 
     MiniBatch mini_batch;
diff --git a/src/model/policy.cpp b/src/model/policy.cpp
@@ -1,4 +1,3 @@
-#include <spdlog/spdlog.h>
 #include <torch/torch.h>
 
 #include "cpprl/model/policy.h"
@@ -35,8 +34,7 @@ PolicyImpl::PolicyImpl(ActionSpace action_space, std::shared_ptr<NNBase> base)
     }
     else
     {
-        spdlog::error("Action space {} not supported", action_space.type);
-        throw std::exception();
+        throw std::runtime_error("Action space " + action_space.type + " not supported");
     }
     register_module("output", output_layer);
 }
diff --git a/src/storage.cpp b/src/storage.cpp
@@ -2,7 +2,6 @@
 #include <vector>
 
 #include <c10/util/ArrayRef.h>
-#include <spdlog/spdlog.h>
 #include <torch/torch.h>
 
 #include "cpprl/generators/feed_forward_generator.h"
@@ -97,13 +96,14 @@ std::unique_ptr<Generator> RolloutStorage::feed_forward_generator(
     auto batch_size = num_processes * num_steps;
     if (batch_size < num_mini_batch)
     {
-        spdlog::error("PPO needs the number of processes ({}) * the number of "
-                      "steps ({}) = {} to be greater than or equal to the number "
-                      "of minibatches ({})",
-                      num_processes,
-                      num_steps,
-                      num_mini_batch);
-        throw std::exception();
+        throw std::runtime_error("PPO needs the number of processes (" +
+                                 std::to_string(num_processes) +
+                                 ") * the number of steps (" +
+                                 std::to_string(num_steps) + ") = " +
+                                 std::to_string(num_processes * num_steps) +
+                                 " to be greater than or equal to the number of minibatches (" +
+                                 std::to_string(num_mini_batch) +
+                                 ")");
     }
     auto mini_batch_size = batch_size / num_mini_batch;
     return std::make_unique<FeedForwardGenerator>(
@@ -143,11 +143,11 @@ std::unique_ptr<Generator> RolloutStorage::recurrent_generator(
     auto num_processes = actions.size(1);
     if (num_processes < num_mini_batch)
     {
-        spdlog::error("PPO needs the number of processes ({}) to be greater than or"
-                      " equal to the number of minibatches ({})",
-                      num_processes,
-                      num_mini_batch);
-        throw std::exception();
+        throw std::runtime_error("PPO needs the number of processes (" +
+                                 std::to_string(num_processes) +
+                                 ") to be greater than or equal to the number of minibatches (" +
+                                 std::to_string(num_mini_batch) +
+                                 ")");
     }
     return std::make_unique<RecurrentGenerator>(
         num_processes,
diff --git a/src/third_party/doctest.cpp b/src/third_party/doctest.cpp
@@ -1,11 +1,3 @@
-#define DOCTEST_CONFIG_IMPLEMENT
+#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
 
-#include <spdlog/spdlog.h>
-
-#include "third_party/doctest.h"
-
-int main(int argc, char **argv)
-{
-    spdlog::set_level(spdlog::level::off);
-    return doctest::Context(argc, argv).run();
-}
+#include "third_party/doctest.h"

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,5 @@`
`1`	`1`	`#include <ATen/core/Reduction.h>`
`2`	`2`	`#include <c10/util/ArrayRef.h>`
`3`		`-#include <spdlog/spdlog.h>`
`4`	`3`	`#include <torch/torch.h>`
`5`	`4`
`6`	`5`	`#include "cpprl/distributions/bernoulli.h"`
`@@ -13,15 +12,14 @@ Bernoulli::Bernoulli(const torch::Tensor *probs,`
`13`	`12`	`{`
`14`	`13`	`if ((probs == nullptr) == (logits == nullptr))`
`15`	`14`	`{`
`16`		`- spdlog::error("Either probs or logits is required, but not both");`
`17`		`- throw std::exception();`
	`15`	`+ throw std::runtime_error("Either probs or logits is required, but not both");`
`18`	`16`	`}`
`19`	`17`
`20`	`18`	`if (probs != nullptr)`
`21`	`19`	`{`
`22`	`20`	`if (probs->dim() < 1)`
`23`	`21`	`{`
`24`		`- throw std::exception();`
	`22`	`+ throw std::runtime_error("Probabilities tensor must have at least one dimension");`
`25`	`23`	`}`
`26`	`24`	`this->probs = *probs;`
`27`	`25`	`// 1.21e-7 is used as the epsilon to match PyTorch's Python results as closely`
`@@ -33,7 +31,7 @@ Bernoulli::Bernoulli(const torch::Tensor *probs,`
`33`	`31`	`{`
`34`	`32`	`if (logits->dim() < 1)`
`35`	`33`	`{`
`36`		`- throw std::exception();`
	`34`	`+ throw std::runtime_error("Logits tensor must have at least one dimension");`
`37`	`35`	`}`
`38`	`36`	`this->logits = *logits;`
`39`	`37`	`this->probs = torch::sigmoid(*logits);`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`#include <c10/util/ArrayRef.h>`
`2`		`-#include <spdlog/spdlog.h>`
`3`	`2`	`#include <torch/torch.h>`
`4`	`3`
`5`	`4`	`#include "cpprl/distributions/categorical.h"`
`@@ -12,15 +11,14 @@ Categorical::Categorical(const torch::Tensor *probs,`
`12`	`11`	`{`
`13`	`12`	`if ((probs == nullptr) == (logits == nullptr))`
`14`	`13`	`{`
`15`		`- spdlog::error("Either probs or logits is required, but not both");`
`16`		`- throw std::exception();`
	`14`	`+ throw std::runtime_error("Either probs or logits is required, but not both");`
`17`	`15`	`}`
`18`	`16`
`19`	`17`	`if (probs != nullptr)`
`20`	`18`	`{`
`21`	`19`	`if (probs->dim() < 1)`
`22`	`20`	`{`
`23`		`- throw std::exception();`
	`21`	`+ throw std::runtime_error("Probabilities tensor must have at least one dimension");`
`24`	`22`	`}`
`25`	`23`	`this->probs = *probs / probs->sum(-1, true);`
`26`	`24`	`// 1.21e-7 is used as the epsilon to match PyTorch's Python results as closely`
`@@ -32,7 +30,7 @@ Categorical::Categorical(const torch::Tensor *probs,`
`32`	`30`	`{`
`33`	`31`	`if (logits->dim() < 1)`
`34`	`32`	`{`
`35`		`- throw std::exception();`
	`33`	`+ throw std::runtime_error("Logits tensor must have at least one dimension");`
`36`	`34`	`}`
`37`	`35`	`this->logits = *logits - logits->logsumexp(-1, true);`
`38`	`36`	`this->probs = torch::softmax(this->logits, -1);`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,6 @@`
`1`	`1`	`#include <algorithm>`
`2`	`2`	`#include <vector>`
`3`	`3`
`4`		`-#include <spdlog/spdlog.h>`
`5`	`4`	`#include <torch/torch.h>`
`6`	`5`
`7`	`6`	`#include "cpprl/generators/feed_forward_generator.h"`
`@@ -44,10 +43,7 @@ MiniBatch FeedForwardGenerator::next()`
`44`	`43`	`{`
`45`	`44`	`if (index >= indices.size(0))`
`46`	`45`	`{`
`47`		`- spdlog::error("No minibatches left in generator. Index {}, minibatch "`
`48`		`- "count: {}.",`
`49`		`- index, indices.size(0));`
`50`		`- throw std::exception();`
	`46`	`+ throw std::runtime_error("No minibatches left in generator.");`
`51`	`47`	`}`
`52`	`48`
`53`	`49`	`MiniBatch mini_batch;`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-#include <spdlog/spdlog.h>`
`2`	`1`	`#include <torch/torch.h>`
`3`	`2`
`4`	`3`	`#include "cpprl/model/policy.h"`
`@@ -35,8 +34,7 @@ PolicyImpl::PolicyImpl(ActionSpace action_space, std::shared_ptr<NNBase> base)`
`35`	`34`	`}`
`36`	`35`	`else`
`37`	`36`	`{`
`38`		`- spdlog::error("Action space {} not supported", action_space.type);`
`39`		`- throw std::exception();`
	`37`	`+ throw std::runtime_error("Action space " + action_space.type + " not supported");`
`40`	`38`	`}`
`41`	`39`	`register_module("output", output_layer);`
`42`	`40`	`}`