Fix some program options bugs

nluka · May 17, 2023 · aab1263 · aab1263
1 parent 57c975b
commit aab1263
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 69 deletions.
diff --git a/Makefile b/Makefile
@@ -27,11 +27,11 @@ OBJS = $(patsubst $(SRC_DIR)/%.cpp, $(BIN_DIR)/%.o, $(SRCS))
 DEPS = $(OBJS:.o=.d)
 
 # Rules
-.PHONY: default all clean
+.PHONY: default toolchain clean
 
-default: all
+default: toolchain
 
-all: next_cluster make_image make_states simulate_one simulate_many file_write_benchmark
+toolchain: next_cluster make_image make_states simulate_one simulate_many
 
 core = $(addprefix $(BIN_DIR)/, fregex.o logger.o pgm8.o program_options.o simulation_misc.o simulation_parse_state.o simulation_run.o simulation_save_state.o util.o term.o)
 

diff --git a/scripts/cluster.py b/scripts/cluster.py
@@ -12,10 +12,10 @@
 make_states_executable_path = "bin/release/make_states"
 simulate_many_executable_path = "bin/release/simulate_many"
 
-width=1000
-height=1000
-col=500
-row=500
+width=500
+height=500
+col=250
+row=250
 gen_limit="1'000'000'000"
 out_dir = None
 sim_count = None
@@ -41,8 +41,16 @@ def ctrl_c_handler(sig, frame):
   out_dir = sys.argv[1]
   sim_count = sys.argv[2]
 
-  proc = subprocess.Popen([next_cluster_executable_path, out_dir],
-                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+  arguments = [
+    next_cluster_executable_path,
+    out_dir
+  ]
+
+  proc = subprocess.Popen(
+    arguments,
+    stdout=subprocess.PIPE,
+    stderr=subprocess.PIPE
+  )
 
   # Wait for the subprocess to finish and capture the exit code
   next_cluster_exit_code = proc.wait()
@@ -54,24 +62,32 @@ def ctrl_c_handler(sig, frame):
 
   master_fd, slave_fd = pty.openpty()
 
+  arguments = [
+    make_states_executable_path,
+    "-N", f"{sim_count}",
+    "-o", f"{out_dir}/cluster{cluster}",
+    "-n", "randwords,2",
+    "-m", "50", "-M", "50",
+    "-t", "LR",
+    "-s", "rand",
+    "-w", f"{width}", "-h", f"{height}",
+    "-x", f"{col}", "-y", f"{row}",
+    "-O", "NESW",
+    "-g", "fill=0",
+    "-W", "resources/words.txt",
+    "-c",
+  ]
+
+  # print("[" + ", ".join(f'"{arg}"' for arg in arguments[1:]) + "]")
+
   proc = subprocess.Popen(
-    [
-      make_states_executable_path,
-      "-N", f"{sim_count}",
-      "-o", f"{out_dir}/cluster{cluster}",
-      "-n", "randwords,2",
-      "-m", "256", "-M", "256",
-      "-t", "LR",
-      "-s", "rand",
-      "-w", f"{width}", "-h", f"{height}",
-      "-x", f"{col}", "-y", f"{row}",
-      "-O", "NESW",
-      "-g", "fill=0",
-      "-W", "resources/words.txt",
-      "-c",
-    ],
-    stdout=slave_fd, stderr=slave_fd, text=True,
-    bufsize=1, close_fds=True, universal_newlines=True
+    arguments,
+    stdout=slave_fd,
+    stderr=slave_fd,
+    text=True,
+    bufsize=1,
+    close_fds=True,
+    universal_newlines=True
   )
 
   os.close(slave_fd)
@@ -98,20 +114,28 @@ def ctrl_c_handler(sig, frame):
 
   master_fd, slave_fd = pty.openpty()
 
+  arguments = [
+    simulate_many_executable_path,
+    # "-T", "2",
+    # "-Q", "50",
+    "-L", f"{out_dir}/cluster{cluster}/log.txt",
+    "-S", f"{out_dir}/cluster{cluster}",
+    "-g", f"{gen_limit}",
+    "-f", "raw",
+    "-o", f"{out_dir}/cluster{cluster}",
+    "-s", "-y", "-l", "-C"
+  ]
+
+  # print("[" + ", ".join(f'"{arg}"' for arg in arguments[1:]) + "]")
+
   proc = subprocess.Popen(
-    [
-      simulate_many_executable_path,
-      # "-T" "6"
-      "-Q", "50",
-      "-L", f"{out_dir}/cluster{cluster}/log.txt",
-      "-S", f"{out_dir}/cluster{cluster}",
-      "-g", f"{gen_limit}",
-      "-f", "raw",
-      "-o", f"{out_dir}/cluster{cluster}",
-      "-s", "-y", "-l", "-C"
-    ],
-    stdout=slave_fd, stderr=slave_fd, text=True,
-    bufsize=1, close_fds=True, universal_newlines=True
+    arguments,
+    stdout=slave_fd,
+    stderr=slave_fd,
+    text=True,
+    bufsize=1,
+    close_fds=True,
+    universal_newlines=True
   )
 
   os.close(slave_fd)
@@ -131,5 +155,7 @@ def ctrl_c_handler(sig, frame):
 
   if (simulate_many_exit_code != 0):
     print(f"simulate_many exited with code {simulate_many_exit_code}")
+    if (simulate_many_exit_code == -9):
+      print(f"SIGKILL, you may not have enough memory")
 
   colorama.deinit()
diff --git a/src/logger.cpp b/src/logger.cpp
@@ -184,7 +184,7 @@ void logger::flush() {
   }
 
   for (auto const &evt : s_events) {
-    if (!file) {
+    if (file.has_value() && !file.value()) {
       throw std::runtime_error("logger::flush failed - bad file");
     }
 

diff --git a/src/program_options.cpp b/src/program_options.cpp
@@ -849,9 +849,7 @@ void po::parse_simulate_one_options(
 
   {
     option const opt = simulate_one::log_file_path();
-    auto log_file_path = out.sim.create_logs
-      ? get_required_option<std::string>(opt, vm, errors)
-      : get_nonrequired_option<std::string>(opt, vm, errors);
+    auto log_file_path = get_nonrequired_option<std::string>(opt, vm, errors);
 
     if (log_file_path.has_value()) {
       if (!util::file_is_openable(log_file_path.value()))
@@ -903,10 +901,8 @@ void po::parse_simulate_many_options(
   }
 
   {
-    option const opt = simulate_one::log_file_path();
-    auto log_file_path = out.sim.create_logs
-      ? get_required_option<std::string>(opt, vm, errors)
-      : get_nonrequired_option<std::string>(opt, vm, errors);
+    option const opt = simulate_many::log_file_path();
+    auto log_file_path = get_nonrequired_option<std::string>(opt, vm, errors);
 
     if (log_file_path.has_value()) {
       if (!util::file_is_openable(log_file_path.value()))

diff --git a/src/simulate_many_main.cpp b/src/simulate_many_main.cpp
@@ -128,25 +128,33 @@ try
 
   // parse state files into initial simulation states and add them to the simulation_queue
   std::thread producer_thread([&]() {
+
     for (i64 i = i64(state_files.size()) - 1; i >= 0; --i) {
       sem_empty.acquire();
       {
         std::scoped_lock sim_q_lock(simulation_queue_mutex);
         errors_t errors{};
         std::string const path_str = state_files[u64(i)].generic_string();
 
-        auto state = simulation::parse_state(
-          util::extract_txt_file_contents(path_str.c_str(), false),
-          fs::path(s_options.state_dir_path),
-          errors);
-
-        if (!errors.empty()) {
-          if (s_options.any_logging_enabled()) {
-            std::string const err = make_str("failed to parse %s: %s", path_str.c_str(), util::stringify_errors(errors).c_str());
-            logger::log(logger::event_type::ERROR, "%s", err.c_str());
+        simulation::state state;
+        try {
+          state = simulation::parse_state(
+            util::extract_txt_file_contents(path_str.c_str(), false),
+            fs::path(s_options.state_dir_path),
+            errors);
+
+          if (!errors.empty()) {
+            if (s_options.any_logging_enabled()) {
+              std::string const err = make_str("failed to parse %s: %s", path_str.c_str(), util::stringify_errors(errors).c_str());
+              logger::log(logger::event_type::ERROR, "%s", err.c_str());
+            }
+          } else {
+            simulation_queue.emplace_back(simulation::extract_name_from_json_state_path(path_str), state);
           }
-        } else {
-          simulation_queue.emplace_back(simulation::extract_name_from_json_state_path(path_str), state);
+        } catch (std::exception const &except) {
+          logger::log(logger::event_type::ERROR, "%s", except.what());
+        } catch (...) {
+          logger::log(logger::event_type::ERROR, "catch (...) in producer_thread");
         }
       }
       sem_full.release();
@@ -164,7 +172,7 @@ try
         s_options.sim.image_format,
         s_options.sim.save_path,
         s_options.sim.save_final_state,
-        s_options.sim.create_logs,
+        s_options.any_logging_enabled(),
         s_options.sim.save_image_only,
         &num_simulations_processed,
         total);
@@ -196,10 +204,14 @@ try
   std::thread consumer_thread([&]() {
     for (u64 i = 0; i < state_files.size(); ++i) {
       sem_full.acquire();
-      {
+      try {
         std::scoped_lock sim_q_lock(simulation_queue_mutex);
         t_pool.push_task(simulation_task, std::move(simulation_queue.back()), state_files.size());
         simulation_queue.pop_back();
+      } catch (std::exception const &except) {
+        logger::log(logger::event_type::ERROR, "%s", except.what());
+      } catch (...) {
+        logger::log(logger::event_type::ERROR, "catch (...) in consumer_thread");
       }
       sem_empty.release();
     }
@@ -232,7 +244,10 @@ try
     percent_iteration = ( f64(nanos_spent_iterating) / f64(nanos_spent_iterating + nanos_spent_saving) ) * 100.0,
     percent_saving    = ( f64(nanos_spent_saving   ) / f64(nanos_spent_iterating + nanos_spent_saving) ) * 100.0;
 
-  std::printf("-----------------------------------\n");
+  if (s_options.any_logging_enabled()) {
+    // print separator between logs and statistics
+    std::printf("-----------------------------------\n");
+  }
   std::printf("Avg Mgens/sec : %.2lf\n", mega_gens_per_sec);
   std::printf("Avg I/S Ratio : %.2lf / %.2lf\n",
       std::isnan(percent_iteration) ? 0.0 : percent_iteration,

diff --git a/src/simulation_parse_state.cpp b/src/simulation_parse_state.cpp
@@ -309,7 +309,7 @@ b8 try_to_parse_and_set_grid_state(
     try {
       state.grid = new u8[num_pixels];
     } catch (std::bad_alloc const &) {
-      add_err(make_str("failed to allocate %zu bytes for grid", num_pixels * sizeof(u8)));
+      add_err(make_str("unable to allocate %zu bytes for grid", num_pixels * sizeof(u8)));
       return false;
     }
 
@@ -320,8 +320,7 @@ b8 try_to_parse_and_set_grid_state(
     fs::path const img_path = dir / grid_state;
 
     if (!fs::exists(img_path)) {
-      add_err(
-        make_str("bad grid_state, file \"%s\" does not exist", grid_state.c_str()));
+      add_err(make_str("bad grid_state, file \"%s\" does not exist", grid_state.c_str()));
       return false;
     }
 
@@ -358,7 +357,7 @@ b8 try_to_parse_and_set_grid_state(
     try {
       state.grid = new u8[num_pixels];
     } catch (std::bad_alloc const &) {
-      add_err(make_str("failed to allocate %zu bytes for grid", num_pixels * sizeof(u8)));
+      add_err(make_str("unable to allocate %zu bytes for grid", num_pixels * sizeof(u8)));
       return false;
     }
 

diff --git a/src/testing_main.cpp b/src/testing_main.cpp
@@ -579,12 +579,11 @@ i32 main()
         "-S", "testing/valid_dir/valid_regular_file",
         "-g", "0",
         "-f", "unknown",
-        "-l", // therefore -L is required
+        "-l",
         "-v", "10", // therefore -o is required
       };
 
       errors_t expected_errors {
-        "-L [ --log_file_path ] required",
         "-o [ --save_path ] required",
         "-f [ --image_format ] must be one of raw|plain",
       };
@@ -826,12 +825,10 @@ i32 main()
         "-S", "testing/valid_dir",
         "-g", "0",
         "-f", "unknown",
-        "-l", // therefore -L is required
         "-v", "10", // therefore -o is required
       };
 
       errors_t expected_errors {
-        "-L [ --log_file_path ] required",
         "-o [ --save_path ] required",
         "-f [ --image_format ] must be one of raw|plain",
       };