merge classical and by batch benders

src/cpp/benders/benders_core/include/BendersBase.h

@@ -45,6 +45,7 @@ class BendersBase {
   CurrentIterationData _data;
   VariableMap master_variable_map_;
   CouplingMap coupling_map_;
+  std::shared_ptr<MathLoggerDriver> mathLoggerDriver_;
 
  protected:
   virtual void free() = 0;

src/cpp/benders/benders_core/include/common.h

@@ -49,7 +49,7 @@ typedef std::vector<ActiveCut> ActiveCutStorage;
 typedef std::pair<std::string, std::string> mps_coupling;
 typedef std::list<mps_coupling> mps_coupling_list;
 
-enum class BENDERSMETHOD { BENDERS, BENDERSBYBATCH, MERGEMPS };
+enum class BENDERSMETHOD { BENDERS, BENDERSBYBATCH };
 
 struct Predicate {
   bool operator()(PointPtr const &lhs, PointPtr const &rhs) const {

src/cpp/benders/factories/BendersFactory.cpp

@@ -1,8 +1,7 @@
 
-#include "BendersFactory.h"
-
 #include <filesystem>
 
+#include "BendersFactory.h"
 #include "LogUtils.h"
 #include "LoggerFactories.h"
 #include "StartUp.h"
@@ -12,6 +11,14 @@
 #include "gflags/gflags.h"
 #include "glog/logging.h"
 
+BENDERSMETHOD DeduceBenderMethod(size_t coupling_map_size, size_t batch_size) {
+  auto method = (batch_size == 0 || batch_size == coupling_map_size - 1)
+                    ? BENDERSMETHOD::BENDERS
+                    : BENDERSMETHOD::BENDERSBYBATCH;
+
+  return method;
+}
+
 int RunBenders(char** argv, const std::filesystem::path& options_file,
                mpi::environment& env, mpi::communicator& world) {
   // Read options, needed to have options.OUTPUTROOT
@@ -39,11 +46,10 @@ int RunBenders(char** argv, const std::filesystem::path& options_file,
         std::filesystem::path(options.OUTPUTROOT) / "benders_solver.log";
 
     Writer writer;
-    const auto couplig_map = build_input(benders_options.STRUCTURE_FILE);
-    const auto method = (options.BATCH_SIZE == 0 ||
-                         options.BATCH_SIZE == couplig_map.size() - 1)
-                            ? BENDERSMETHOD::BENDERS
-                            : BENDERSMETHOD::BENDERSBYBATCH;
+    const auto coupling_map = build_input(benders_options.STRUCTURE_FILE);
+    const auto method =
+        DeduceBenderMethod(coupling_map.size(), options.BATCH_SIZE);
+
     if (world.rank() == 0) {
       auto benders_log_console = benders_options.LOG_LEVEL > 0;
       auto logger_factory =
@@ -63,16 +69,21 @@ int RunBenders(char** argv, const std::filesystem::path& options_file,
       math_log_driver = MathLoggerFactory::get_void_logger();
     }
 
-    world.barrier();
+    benders_loggers.AddLogger(logger);
+    benders_loggers.AddLogger(math_log_driver);
     pBendersBase benders;
-    if (method == BENDERSMETHOD::BENDERS) {
-      benders = std::make_shared<BendersMpi>(benders_options, logger, writer,
-                                             env, world);
-    } else {
-      benders = std::make_shared<BendersByBatch>(benders_options, logger,
-                                                 writer, env, world);
+    switch (method) {
+      case BENDERSMETHOD::BENDERS:
+        benders = std::make_shared<BendersMpi>(benders_options, logger, writer,
+                                               env, world, math_log_driver);
+        break;
+      case BENDERSMETHOD::BENDERSBYBATCH:
+        benders = std::make_shared<BendersByBatch>(
+            benders_options, logger, writer, env, world, math_log_driver);
+        break;
     }
-    benders->set_input_map(couplig_map);
+
+    benders->set_input_map(coupling_map);
     std::ostringstream oss_l = start_message(options, benders->BendersName());
     oss_l << std::endl;
     benders_loggers.display_message(oss_l.str());

src/cpp/benders/factories/include/BendersFactory.h

@@ -5,7 +5,6 @@
 #include "BendersSequential.h"
 #include "ILogger.h"
 #include "OutputWriter.h"
-#include "common.h"
 
 class BendersMainFactory {
  private:

src/cpp/full_run/FullRunOptionsParser.cpp

@@ -13,5 +13,5 @@ FullRunOptionsParser::FullRunOptionsParser() : ProblemGenerationExeOptions() {
       "path to json solution file");
 }
 void FullRunOptionsParser::Parse(unsigned int argc, const char* const* argv) {
-  OptionsParser::Parse(argc, argv);
+  ProblemGenerationExeOptions::Parse(argc, argv);
 }

tests/CMakeLists.txt

@@ -12,9 +12,9 @@ find_python_module(pytest)
 find_python_module(numpy)
 
 if (PYTHON_MODULE_pytest_FOUND AND PYTHON_MODULE_numpy_FOUND)
-        set(xpress_avalaible "False")
+        set(xpress_avalaible "")
         if(${XPRESS})
-	        set(xpress_avalaible "True")
+	        set(xpress_avalaible "--xpress")
         endif() 
     # Python unit test
     add_test(
@@ -89,7 +89,7 @@ if (PYTHON_MODULE_pytest_FOUND AND PYTHON_MODULE_numpy_FOUND)
     # benders end to end tests
     add_test(
             NAME sequential
-            COMMAND Python3::Interpreter -m pytest --installDir=${XPANSION_INSTALL_DIR} --xpress=${xpress_avalaible} test_bendersSequentialEndToEnd.py
+            COMMAND Python3::Interpreter -m pytest --installDir=${XPANSION_INSTALL_DIR} ${xpress_avalaible} test_bendersSequentialEndToEnd.py
             WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/end_to_end/benders
     )
     set_property(TEST sequential PROPERTY LABELS benders benders-sequential end_to_end)
@@ -101,7 +101,7 @@ if (PYTHON_MODULE_pytest_FOUND AND PYTHON_MODULE_numpy_FOUND)
     set_property(TEST sequential_restart PROPERTY LABELS benders benders-sequential end_to_end restart)
     add_test(
             NAME mpibenders
-            COMMAND Python3::Interpreter -m pytest --allow_run_as_root=${ALLOW_RUN_AS_ROOT} --installDir=${XPANSION_INSTALL_DIR} --xpress=${xpress_avalaible} test_bendersmpibendersEndToEnd.py
+            COMMAND Python3::Interpreter -m pytest --allow_run_as_root=${ALLOW_RUN_AS_ROOT} --installDir=${XPANSION_INSTALL_DIR} ${xpress_avalaible} test_bendersmpibendersEndToEnd.py
             WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/end_to_end/benders
     )
     set_property(TEST mpibenders PROPERTY LABELS benders benders-mpi end_to_end)

tests/cpp/full_run/FullRunTest.cpp

@@ -38,12 +38,11 @@ TEST_P(FullRunOptionsParserTest, ThatBendersOptionFileIsRequired) {
   }
 }
 
-TEST_F(FullRunOptionsParserTest, ThatSolutionOptionIsRequired) {
-  const char argv0[] = "full_run.exe";
-  const char argv1[] = "--archive";
-  const char argv2[] = "something";
-  const char argv3[] = "--output";
-  const char argv4[] = "something";
+TEST_P(FullRunOptionsParserTest, ThatSolutionOptionIsRequired) {
+  auto params = GetParam();
+  std::vector<const char*> pargs;
+  std::ranges::transform(params, std::back_inserter(pargs),
+                         [](const std::string& s) { return s.data(); });
   const char argv5[] = "--benders_options";
   const char argv6[] = "something";
   pargs.push_back(argv5);
@@ -55,24 +54,34 @@ TEST_F(FullRunOptionsParserTest, ThatSolutionOptionIsRequired) {
               std::string("the option '--solution' is required but missing"));
   }
 }
+INSTANTIATE_TEST_SUITE_P(args, FullRunOptionsParserTest, params());
 
-TEST_F(FullRunOptionsParserTest, OptionsParsing) {
-  const char argv0[] = "full_run.exe";
-  const char argv1[] = "--archive";
-  const char argv2[] = "/path/to/output.zip";
-  const char argv3[] = "--output";
-  const char argv4[] = "/path/to/output";
+TEST_P(FullRunOptionsParserTestFullPath, OptionsParsing) {
+  auto params = GetParam();
+  std::vector<const char*> pargs;
+  std::ranges::transform(params, std::back_inserter(pargs),
+                         [](const std::string& s) { return s.data(); });
   const char argv5[] = "--benders_options";
-  const char argv6[] = "options.json";
+  const char argv6[] = "something";
+  pargs.push_back(argv5);
+  pargs.push_back(argv6);
   const char argv7[] = "-s";
   const char argv8[] = "/path/to/solution.json";
-  std::vector<const char*> ppargv = {argv0, argv1, argv2, argv3, argv4,
-                                     argv5, argv6, argv7, argv8};
-  full_run_options_options_parser_.Parse(9, ppargv.data());
-  ASSERT_EQ(full_run_options_options_parser_.ArchivePath(),
-            std::filesystem::path(argv2));
+  pargs.push_back(argv7);
+  pargs.push_back(argv8);
+
+  full_run_options_options_parser_.Parse(pargs.size(), pargs.data());
   ASSERT_EQ(full_run_options_options_parser_.BendersOptionsFile(),
             std::filesystem::path(argv6));
   ASSERT_EQ(full_run_options_options_parser_.SolutionFile(),
             std::filesystem::path(argv8));
 }
+
+auto full_path_params() {
+  return ::testing::ValuesIn(std::vector<std::vector<std::string>>{
+      {"full_run.exe", "--archive", "/path/to/output.zip"},
+      {"full_run.exe", "--output", "/path/to/output"},
+  });
+}
+INSTANTIATE_TEST_SUITE_P(args, FullRunOptionsParserTestFullPath,
+                         full_path_params());

tests/end_to_end/benders/conftest.py

@@ -7,7 +7,7 @@ def pytest_addoption(parser):
     parser.addoption("--installDir", action="store",
                      default=build_config_reader.get_install_dir())
     parser.addoption("--allow_run_as_root", action="store", default="")
-    parser.addoption("--xpress", action="store", default="")
+    parser.addoption("--xpress", action="store_false", default=False)
 
 
 @pytest.fixture()

tests/python/test_full_run_driver.py

@@ -46,7 +46,7 @@ def test_sequential_command(self, tmp_path):
         xpansion_output_dir = output_path.parent / \
             (output_path.stem+"-Xpansion")
         expected_command = [self.full_run_exe, "--benders_options", self.benders_driver_options_file,
-                            "-s", str(json_file_path), "-a", str(output_path), "-o", str(xpansion_output_dir), "-f", "integer", "-e", self.pb_gen_data.additional_constraints]
+                            "-s", str(json_file_path), "-a", str(output_path), "-f", "integer", "-e", self.pb_gen_data.additional_constraints]
 
         command = full_run_driver.full_command()
         assert len(expected_command) == len(command)
@@ -80,7 +80,7 @@ def test_mpi_command(self, tmp_path):
         xpansion_output_dir = output_path.parent / \
             (output_path.stem+"-Xpansion")
         expected_command = [benders_driver.MPI_LAUNCHER, "-n", str(benders_n_mpi), self.full_run_exe, "--benders_options", self.benders_driver_options_file,
-                            "-s", str(json_file_path), "-a", str(output_path), "-o", str(xpansion_output_dir), "-f", "integer", "-e", self.pb_gen_data.additional_constraints]
+                            "-s", str(json_file_path), "-a", str(output_path), "-f", "integer", "-e", self.pb_gen_data.additional_constraints]
 
         command = full_run_driver.full_command()