huch, quite a logical mistake..

valhalla · kevinkreiser · Jul 4, 2023 · Jun 17, 2023 · Jun 17, 2023 · Jun 17, 2023
commit c4a5e50d155c99e370d8aae210ee162d69f4b252
diff --git a/src/mjolnir/valhalla_add_elevation.cc b/src/mjolnir/valhalla_add_elevation.cc
@@ -22,7 +22,6 @@ using namespace valhalla::mjolnir;
 
 boost::property_tree::ptree config;
 std::vector<std::string> tiles;
-uint32_t num_threads = 0;
 
 /*
  * This tool downloads elevations from remote storage for each provided tile.
@@ -81,7 +80,7 @@ bool parse_arguments(int argc, char** argv) {
       ("v,version", "Print the version of this software.")
       ("c,config", "Path to the configuration file.",  opt::value<std::string>())
       ("t,tiles", "Tiles to add elevations to", opt::value<std::vector<std::string>>(tiles))
-      ("j,concurrency", "Number of threads to use. Defaults to all threads.", opt::value<uint32_t>(num_threads));
+      ("j,concurrency", "Number of threads to use. Defaults to all threads.", opt::value<uint32_t>());
     // clang-format on
 
     auto result = options.parse(argc, argv);
@@ -94,9 +93,11 @@ bool parse_arguments(int argc, char** argv) {
       return false;
     }
 
-    if (num_threads) {
-      config.put<unsigned int>("mjolnir.concurrency", num_threads);
-    }
+    config.put<uint32_t>("mjolnir.concurrency",
+                         result.count("concurrency")
+                             ? result["concurrency"].as<uint32_t>()
+                             : config.get<uint32_t>("mjolnir.concurrency",
+                                                    std::thread::hardware_concurrency()));
 
     if (!result.count("tiles")) {
       std::cerr << "Tile file is required\n\n" << options.help() << "\n\n";

diff --git a/src/mjolnir/valhalla_add_predicted_traffic.cc b/src/mjolnir/valhalla_add_predicted_traffic.cc
@@ -36,7 +36,6 @@ namespace bpt = boost::property_tree;
 // args
 boost::property_tree::ptree config;
 filesystem::path traffic_tile_dir;
-unsigned int num_threads;
 bool summary = false;
 
 namespace {
@@ -253,11 +252,11 @@ bool ParseArguments(int argc, char* argv[]) {
     options.add_options()
       ("h,help", "Print this help message.")
       ("v,version", "Print the version of this software.")
-      ("j,concurrency", "Number of threads to use.", cxxopts::value<unsigned int>(num_threads))
+      ("j,concurrency", "Number of threads to use.", cxxopts::value<unsigned int>())
       ("c,config", "Path to the json configuration file.", cxxopts::value<std::string>())
       ("i,inline-config", "Inline json config.", cxxopts::value<std::string>())
       ("s,summary", "Output summary information about traffic coverage for the tile set", cxxopts::value<bool>(summary))
-      ("t,traffic_tile_dir", "positional argument", cxxopts::value<std::string>());
+      ("t,traffic-tile-dir", "positional argument", cxxopts::value<std::string>());
     // clang-format on
 
     options.parse_positional({"traffic-tile-dir"});
@@ -274,11 +273,11 @@ bool ParseArguments(int argc, char* argv[]) {
       exit(0);
     }
 
-    if (!result.count("traffic_tile_dir")) {
+    if (!result.count("traffic-tile-dir")) {
       std::cout << "You must provide a tile directory to read the csv tiles from.\n";
       return false;
     }
-    traffic_tile_dir = filesystem::path(result["traffic_tile_dir"].as<std::string>());
+    traffic_tile_dir = filesystem::path(result["traffic-tile-dir"].as<std::string>());
 
     // Read the config file
     if (result.count("inline-config")) {
@@ -293,9 +292,11 @@ bool ParseArguments(int argc, char* argv[]) {
       return false;
     }
 
-    if (num_threads) {
-      config.put<unsigned int>("mjolnir.concurrency", num_threads);
-    }
+    config.put<uint32_t>("mjolnir.concurrency",
+                         result.count("concurrency")
+                             ? result["concurrency"].as<uint32_t>()
+                             : config.get<uint32_t>("mjolnir.concurrency",
+                                                    std::thread::hardware_concurrency()));
 
     return true;
   } catch (cxxopts::OptionException& e) {
@@ -341,11 +342,10 @@ int main(int argc, char** argv) {
   std::random_device rd;
   std::shuffle(traffic_tiles.begin(), traffic_tiles.end(), std::mt19937(rd()));
 
+  auto num_threads = std::max(1U, config.get<uint32_t>("mjolnir.concurrency"));
   LOG_INFO("Adding predicted traffic with " + std::to_string(num_threads) + " threads");
   std::vector<std::shared_ptr<std::thread>> threads(num_threads);
 
-  std::cout << traffic_tile_dir << std::endl;
-
   LOG_INFO("Parsing speeds from " + std::to_string(traffic_tiles.size()) + " tiles.");
   size_t floor = traffic_tiles.size() / threads.size();
   size_t at_ceiling = traffic_tiles.size() - (threads.size() * floor);

diff --git a/src/mjolnir/valhalla_assign_speeds.cc b/src/mjolnir/valhalla_assign_speeds.cc
@@ -79,7 +79,6 @@ int main(int argc, char** argv) {
   // args
   filesystem::path config_file_path;
   bpt::ptree config;
-  uint32_t num_threads = 0;
 
   try {
     // clang-format off
@@ -93,7 +92,7 @@ int main(int argc, char** argv) {
       ("v,version", "Print the version of this software.")
       ("c,config", "Path to the json configuration file.", cxxopts::value<std::string>())
       ("i,inline-config", "Inline JSON config", cxxopts::value<std::string>())
-      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>(num_threads));
+      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>());
     // clang-format on
 
     auto result = options.parse(argc, argv);
@@ -122,9 +121,11 @@ int main(int argc, char** argv) {
       return EXIT_FAILURE;
     }
 
-    if (num_threads) {
-      config.put<unsigned int>("mjolnir.concurrency", num_threads);
-    }
+    config.put<uint32_t>("mjolnir.concurrency",
+                         result.count("concurrency")
+                             ? result["concurrency"].as<uint32_t>()
+                             : config.get<uint32_t>("mjolnir.concurrency",
+                                                    std::thread::hardware_concurrency()));
 
     if (!result.count("config")) {
       std::cout << "You must provide a config for loading and modifying tiles.\n";
@@ -158,9 +159,7 @@ int main(int argc, char** argv) {
   std::shuffle(tilequeue.begin(), tilequeue.end(), std::mt19937(3));
 
   // spawn threads to modify the tiles
-  auto concurrency =
-      std::max(static_cast<unsigned int>(1),
-               config.get<unsigned int>("mjolnir.concurrency", std::thread::hardware_concurrency()));
+  auto concurrency = std::max(1U, config.get<uint32_t>("mjolnir.concurrency"));
   std::vector<std::shared_ptr<std::thread>> threads(concurrency);
   std::list<std::promise<std::pair<size_t, size_t>>> results;
   std::mutex lock;

diff --git a/src/mjolnir/valhalla_build_statistics.cc b/src/mjolnir/valhalla_build_statistics.cc
@@ -36,7 +36,7 @@ using namespace valhalla::baldr;
 using namespace valhalla::mjolnir;
 
 filesystem::path config_file_path;
-uint32_t num_threads = 0;
+boost::property_tree::ptree pt;
 
 namespace {
 
@@ -583,7 +583,7 @@ bool ParseArguments(int argc, char* argv[]) {
       ("h,help", "Print this help message")
       ("v,version", "Print the version of this software.")
       ("c,config", "Path to the json configuration file.", cxxopts::value<std::string>())
-      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>(num_threads));
+      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>());
     // clang-format on
 
     auto result = options.parse(argc, argv);
@@ -601,10 +601,18 @@ bool ParseArguments(int argc, char* argv[]) {
     if (result.count("config") &&
         filesystem::is_regular_file(config_file_path =
                                         filesystem::path(result["config"].as<std::string>()))) {
-      return true;
+      rapidjson::read_json(config_file_path.string(), pt);
     } else {
       std::cerr << "Configuration file is required\n\n" << options.help() << "\n\n";
+      return false;
     }
+
+    pt.put<uint32_t>("mjolnir.concurrency",
+                     result.count("concurrency")
+                         ? result["concurrency"].as<uint32_t>()
+                         : pt.get<uint32_t>("mjolnir.concurrency",
+                                            std::thread::hardware_concurrency()));
+    return true;
   } catch (const cxxopts::OptionException& e) {
     std::cout << "Unable to parse command line options because: " << e.what() << std::endl;
   }
@@ -617,14 +625,6 @@ int main(int argc, char** argv) {
     return EXIT_FAILURE;
   }
 
-  // check the type of input
-  boost::property_tree::ptree pt;
-  rapidjson::read_json(config_file_path.string(), pt);
-
-  if (num_threads) {
-    pt.put<unsigned int>("mjolnir.concurrency", num_threads);
-  }
-
   // configure logging
   auto logging_subtree = pt.get_child_optional("mjolnir.logging");
   if (logging_subtree) {

diff --git a/src/mjolnir/valhalla_build_tiles.cc b/src/mjolnir/valhalla_build_tiles.cc
@@ -30,7 +30,6 @@ int main(int argc, char** argv) {
   BuildStage start_stage = BuildStage::kInitialize;
   BuildStage end_stage = BuildStage::kCleanup;
   boost::property_tree::ptree pt;
-  uint32_t num_threads = 0;
 
   try {
 
@@ -50,7 +49,7 @@ int main(int argc, char** argv) {
       ("s,start", "Starting stage of the build pipeline", cxxopts::value<std::string>()->default_value("initialize"))
       ("e,end", "End stage of the build pipeline", cxxopts::value<std::string>()->default_value("cleanup"))
       ("input_files", "positional arguments", cxxopts::value<std::vector<std::string>>(input_files))
-      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>(num_threads));
+      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>());
     // clang-format on
 
     options.parse_positional({"input_files"});
@@ -82,9 +81,11 @@ int main(int argc, char** argv) {
       return EXIT_FAILURE;
     }
 
-    if (num_threads) {
-      pt.put<unsigned int>("mjolnir.concurrency", num_threads);
-    }
+    pt.put<uint32_t>("mjolnir.concurrency",
+                     result.count("concurrency")
+                         ? result["concurrency"].as<uint32_t>()
+                         : pt.get<uint32_t>("mjolnir.concurrency",
+                                            std::thread::hardware_concurrency()));
 
     // configure logging
     auto logging_subtree = pt.get_child_optional("mjolnir.logging");

diff --git a/src/mjolnir/valhalla_convert_transit.cc b/src/mjolnir/valhalla_convert_transit.cc
@@ -9,7 +9,6 @@
 filesystem::path config_file_path;
 boost::property_tree::ptree pt;
 std::vector<valhalla::mjolnir::OneStopTest> onestoptests;
-uint32_t num_threads = 0;
 
 bool ParseArguments(int argc, char* argv[]) {
   try {
@@ -24,7 +23,7 @@ bool ParseArguments(int argc, char* argv[]) {
       ("h,help", "Print this help message.")
       ("v,version", "Print the version of this software.")
       ("c,config", "Path to the json configuration file.", cxxopts::value<std::string>())
-      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>(num_threads))
+      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>())
       ("target_directory", "Path to write transit tiles", cxxopts::value<std::string>())
       ("test_file", "file where tests are written", cxxopts::value<std::string>());
 
@@ -47,14 +46,16 @@ bool ParseArguments(int argc, char* argv[]) {
         filesystem::is_regular_file(config_file_path =
                                         filesystem::path(result["config"].as<std::string>()))) {
       rapidjson::read_json(config_file_path.string(), pt);
-      return true;
     } else {
       std::cerr << "Configuration file is required\n" << options.help() << "\n\n";
+      return false;
     }
 
-    if (num_threads) {
-      pt.put<unsigned int>("mjolnir.concurrency", num_threads);
-    }
+    pt.put<uint32_t>("mjolnir.concurrency",
+                     result.count("concurrency")
+                         ? result["concurrency"].as<uint32_t>()
+                         : pt.get<uint32_t>("mjolnir.concurrency",
+                                            std::thread::hardware_concurrency()));
 
     if (result.count("target_directory")) {
       pt.get_child("mjolnir").erase("transit_dir");
@@ -69,6 +70,7 @@ bool ParseArguments(int argc, char* argv[]) {
       std::sort(onestoptests.begin(), onestoptests.end());
     }
 
+    return true;
   } catch (cxxopts::OptionException& e) {
     std::cerr << "Unable to parse command line options because: " << e.what() << "\n"
               << "This is a bug, please report it at " PACKAGE_BUGREPORT << "\n";
@@ -79,7 +81,7 @@ bool ParseArguments(int argc, char* argv[]) {
               << std::endl;
   }
 
-  return EXIT_FAILURE;
+  return false;
 }
 
 int main(int argc, char** argv) {

diff --git a/src/mjolnir/valhalla_ingest_transit.cc b/src/mjolnir/valhalla_ingest_transit.cc
@@ -5,7 +5,7 @@
 #include <cxxopts.hpp>
 
 filesystem::path config_file_path;
-uint32_t num_threads = 0;
+boost::property_tree::ptree pt;
 
 bool ParseArguments(int argc, char* argv[]) {
   try {
@@ -20,7 +20,7 @@ bool ParseArguments(int argc, char* argv[]) {
       ("h,help", "Print this help message.")
       ("v,version", "Print the version of this software.")
       ("c,config", "Path to the json configuration file.", cxxopts::value<std::string>())
-      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>(num_threads));
+      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>());
     // clang-format on
 
     auto result = options.parse(argc, argv);
@@ -38,16 +38,25 @@ bool ParseArguments(int argc, char* argv[]) {
     if (result.count("config") &&
         filesystem::is_regular_file(config_file_path =
                                         filesystem::path(result["config"].as<std::string>()))) {
-      return true;
+      rapidjson::read_json(config_file_path.string(), pt);
     } else {
       std::cerr << "Configuration file is required\n" << options.help() << "\n\n";
+      return false;
     }
+
+    pt.put<uint32_t>("mjolnir.concurrency",
+                     result.count("concurrency")
+                         ? result["concurrency"].as<uint32_t>()
+                         : pt.get<uint32_t>("mjolnir.concurrency",
+                                            std::thread::hardware_concurrency()));
+
+    return true;
   } catch (cxxopts::OptionException& e) {
     std::cerr << "Unable to parse command line options because: " << e.what() << "\n"
               << "This is a bug, please report it at " PACKAGE_BUGREPORT << "\n";
   }
 
-  return EXIT_FAILURE;
+  return false;
 }
 
 int main(int argc, char** argv) {
@@ -56,14 +65,6 @@ int main(int argc, char** argv) {
     return EXIT_FAILURE;
   }
 
-  // args and config file loading
-  boost::property_tree::ptree pt;
-  rapidjson::read_json(config_file_path.string(), pt);
-
-  if (num_threads) {
-    pt.put<unsigned int>("mjolnir.concurrency", num_threads);
-  }
-
   // spawn threads to download all the tiles returning a list of
   // tiles that ended up having dangling stop pairs
   auto dangling_tiles = valhalla::mjolnir::ingest_transit(pt);

diff --git a/src/mjolnir/valhalla_validate_transit.cc b/src/mjolnir/valhalla_validate_transit.cc
@@ -19,7 +19,7 @@
 using namespace valhalla::mjolnir;
 
 filesystem::path config_file_path;
-uint32_t num_threads = 0;
+boost::property_tree::ptree pt;
 
 bool ParseArguments(int argc, char* argv[]) {
   try {
@@ -35,7 +35,7 @@ bool ParseArguments(int argc, char* argv[]) {
       ("h,help", "Print this help message.")
       ("v,version", "Print the version of this software.")
       ("c,config", "Path to the json configuration file.", cxxopts::value<std::string>())
-      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>(num_threads));
+      ("j,concurrency", "Number of threads to use. Defaults to all threads.", cxxopts::value<uint32_t>());
     // clang-format on
 
     auto result = options.parse(argc, argv);
@@ -53,17 +53,23 @@ bool ParseArguments(int argc, char* argv[]) {
     if (result.count("config") &&
         filesystem::is_regular_file(config_file_path =
                                         filesystem::path(result["config"].as<std::string>()))) {
-      return true;
     } else {
       std::cerr << "Configuration file is required\n\n" << options.help() << "\n\n";
+      return false;
     }
 
-    return false;
+    pt.put<uint32_t>("mjolnir.concurrency",
+                     result.count("concurrency")
+                         ? result["concurrency"].as<uint32_t>()
+                         : pt.get<uint32_t>("mjolnir.concurrency",
+                                            std::thread::hardware_concurrency()));
+
+    return true;
   } catch (const cxxopts::OptionException& e) {
     std::cout << "Unable to parse command line options because: " << e.what() << std::endl;
   }
 
-  return EXIT_FAILURE;
+  return false;
 }
 
 int main(int argc, char** argv) {
@@ -72,14 +78,6 @@ int main(int argc, char** argv) {
     return EXIT_FAILURE;
   }
 
-  // check what type of input we are getting
-  boost::property_tree::ptree pt;
-  rapidjson::read_json(config_file_path.string(), pt);
-
-  if (num_threads) {
-    pt.put<unsigned int>("mjolnir.concurrency", num_threads);
-  }
-
   // configure logging
   auto logging_subtree = pt.get_child_optional("mjolnir.logging");
   if (logging_subtree) {