From 2033c5c9036ac35946db8e8bba46b04a425e3acc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jason=20Mar=C3=A9chal?= <45510813+JasonMarechal25@users.noreply.github.com> Date: Tue, 21 May 2024 09:54:32 +0200 Subject: [PATCH] Fix sonar issues (#819) - Remove FakeWorkerMaqter from tests. Doesn't seems to do anything - Remove dynamique allocation of WorkerMaster. Replace with perfect forwarding arguments from reset_master function to make_shared - Add return value to non void function --- src/cpp/benders/benders_core/BendersBase.cpp | 8 ++--- .../benders_core/OuterLoopInputDataReader.cpp | 12 ++++--- .../benders_core/include/BendersBase.h | 8 +++-- .../include/OuterLoopInputDataReader.h | 2 +- src/cpp/benders/benders_mpi/BendersMPI.cpp | 4 +-- .../benders/benders_mpi/include/BendersMPI.h | 4 +-- .../benders_sequential/BendersSequential.cpp | 4 +-- tests/cpp/benders/benders_sequential_test.cpp | 32 ++----------------- 8 files changed, 25 insertions(+), 49 deletions(-) diff --git a/src/cpp/benders/benders_core/BendersBase.cpp b/src/cpp/benders/benders_core/BendersBase.cpp index 1f1147d92..63f9e8af9 100644 --- a/src/cpp/benders/benders_core/BendersBase.cpp +++ b/src/cpp/benders/benders_core/BendersBase.cpp @@ -720,10 +720,6 @@ std::map BendersBase::get_master_variable_map( return it_master->second; } -void BendersBase::reset_master(WorkerMaster *worker_master) { - _master.reset(worker_master); - master_is_empty_ = false; -} void BendersBase::free_master() { _master->free(); master_is_empty_ = true; @@ -915,10 +911,10 @@ void BendersBase::MasterAddRows( row_names); } void BendersBase::ResetMasterFromLastIteration() { - reset_master(new WorkerMaster(master_variable_map_, LastMasterPath(), + reset_master(master_variable_map_, LastMasterPath(), get_solver_name(), get_log_level(), _data.nsubproblem, solver_log_manager_, - IsResumeMode(), _logger)); + IsResumeMode(), _logger); } bool BendersBase::MasterIsEmpty() const { return master_is_empty_; } diff --git a/src/cpp/benders/benders_core/OuterLoopInputDataReader.cpp b/src/cpp/benders/benders_core/OuterLoopInputDataReader.cpp index 47dc74ac3..8d74b111e 100644 --- a/src/cpp/benders/benders_core/OuterLoopInputDataReader.cpp +++ b/src/cpp/benders/benders_core/OuterLoopInputDataReader.cpp @@ -1,14 +1,16 @@ #include "OuterLoopInputDataReader.h" +#include + using namespace Outerloop; /** * prefix could be := PositiveUnsuppliedEnergy:: or something else necessarily * /!\ body could be := area name or equivalent or nothing */ -OuterLoopPattern::OuterLoopPattern(const std::string &prefix, - const std::string &body) - : prefix_(prefix), body_(body) {} +OuterLoopPattern::OuterLoopPattern(std::string prefix, + std::string body) + : prefix_(std::move(prefix)), body_(std::move(body)) {} /** * just do @@ -119,7 +121,7 @@ namespace YAML { template <> struct convert { static Node encode(const OuterLoopSingleInputData &rhs) { - // + return {}; } static bool decode(const Node &pattern, OuterLoopSingleInputData &rhs) { @@ -152,7 +154,7 @@ struct convert { template <> struct convert { static Node encode(const OuterLoopInputData &rhs) { - // + return {}; } static void DecodePatterns(const Node &patterns, OuterLoopInputData &rhs) { diff --git a/src/cpp/benders/benders_core/include/BendersBase.h b/src/cpp/benders/benders_core/include/BendersBase.h index 67ea04c9f..00df41c99 100644 --- a/src/cpp/benders/benders_core/include/BendersBase.h +++ b/src/cpp/benders/benders_core/include/BendersBase.h @@ -150,11 +150,15 @@ class BendersBase { [[nodiscard]] std::filesystem::path OuterloopOptionsFile() const; [[nodiscard]] LogData bendersDataToLogData( const CurrentIterationData &data) const; - virtual void reset_master(WorkerMaster *worker_master); + template + void reset_master(Args &&...args) { + _master = std::make_shared(std::forward(args)...); + master_is_empty_ = false; + } void free_master(); void free_subproblems(); void AddSubproblem(const std::pair &kvp); - [[nodiscard]] WorkerMasterPtr get_master() const; + [[nodiscard]] virtual WorkerMasterPtr get_master() const; void MatchProblemToId(); /** * for the nth variable name, Subproblems shares the same prefix , only the diff --git a/src/cpp/benders/benders_core/include/OuterLoopInputDataReader.h b/src/cpp/benders/benders_core/include/OuterLoopInputDataReader.h index 3ce2dd650..3a9d6df2f 100644 --- a/src/cpp/benders/benders_core/include/OuterLoopInputDataReader.h +++ b/src/cpp/benders/benders_core/include/OuterLoopInputDataReader.h @@ -41,7 +41,7 @@ class OuterLoopCouldNotReadCriterionField /// @brief lovely class class OuterLoopPattern { public: - explicit OuterLoopPattern(const std::string &prefix, const std::string &body); + explicit OuterLoopPattern(std::string prefix, std::string body); OuterLoopPattern() = default; [[nodiscard]] std::regex MakeRegex() const; [[nodiscard]] const std::string &GetPrefix() const; diff --git a/src/cpp/benders/benders_mpi/BendersMPI.cpp b/src/cpp/benders/benders_mpi/BendersMPI.cpp index faed57825..4c8801d0a 100644 --- a/src/cpp/benders/benders_mpi/BendersMPI.cpp +++ b/src/cpp/benders/benders_mpi/BendersMPI.cpp @@ -49,10 +49,10 @@ void BendersMpi::InitializeProblems() { } void BendersMpi::BuildMasterProblem() { if (_world.rank() == rank_0) { - reset_master(new WorkerMaster(master_variable_map_, get_master_path(), + reset_master(master_variable_map_, get_master_path(), get_solver_name(), get_log_level(), _data.nsubproblem, solver_log_manager_, - IsResumeMode(), _logger)); + IsResumeMode(), _logger); } } /*! diff --git a/src/cpp/benders/benders_mpi/include/BendersMPI.h b/src/cpp/benders/benders_mpi/include/BendersMPI.h index 179af7da7..ceff3d3dc 100644 --- a/src/cpp/benders/benders_mpi/include/BendersMPI.h +++ b/src/cpp/benders/benders_mpi/include/BendersMPI.h @@ -28,9 +28,9 @@ class BendersMpi : public BendersBase { std::shared_ptr mathLoggerDriver); void launch() override; - virtual std::string BendersName() const { return "Benders mpi"; } + std::string BendersName() const override { return "Benders mpi"; } const unsigned int rank_0 = 0; - virtual void ExternalLoopCheckFeasibility() override; + void ExternalLoopCheckFeasibility() override; protected: void free() override; diff --git a/src/cpp/benders/benders_sequential/BendersSequential.cpp b/src/cpp/benders/benders_sequential/BendersSequential.cpp index cc56b7423..57bc50d78 100644 --- a/src/cpp/benders/benders_sequential/BendersSequential.cpp +++ b/src/cpp/benders/benders_sequential/BendersSequential.cpp @@ -26,10 +26,10 @@ BendersSequential::BendersSequential( void BendersSequential::InitializeProblems() { MatchProblemToId(); - reset_master(new WorkerMaster(master_variable_map_, get_master_path(), + reset_master(master_variable_map_, get_master_path(), get_solver_name(), get_log_level(), _data.nsubproblem, solver_log_manager_, - IsResumeMode(), _logger)); + IsResumeMode(), _logger); for (const auto &problem : coupling_map_) { const auto subProblemFilePath = GetSubproblemPath(problem.first); diff --git a/tests/cpp/benders/benders_sequential_test.cpp b/tests/cpp/benders/benders_sequential_test.cpp index 2dd3782c0..bd381c29d 100644 --- a/tests/cpp/benders/benders_sequential_test.cpp +++ b/tests/cpp/benders/benders_sequential_test.cpp @@ -7,27 +7,6 @@ #include "RandomDirGenerator.h" #include "gtest/gtest.h" -class FakeWorkerMaster : public WorkerMaster { - public: - WorkerMasterPtr worker_master; - - FakeWorkerMaster(WorkerMasterPtr worker_master) - : WorkerMaster(worker_master->logger_), worker_master(worker_master){}; - std::vector get_id_nb_units() const override { - return worker_master->get_id_nb_units(); - }; - - void DeactivateIntegrityConstraints() const override { - worker_master->DeactivateIntegrityConstraints(); - }; - - void ActivateIntegrityConstraints() const override { - worker_master->ActivateIntegrityConstraints(); - }; - - SolverAbstract::Ptr solver() const override { return worker_master->_solver; } -}; - class BendersSequentialDouble : public BendersSequential { public: bool parametrized_stop = false; @@ -56,7 +35,7 @@ class BendersSequentialDouble : public BendersSequential { _data.it = parametrized_it; }; - [[nodiscard]] WorkerMasterPtr get_master() const { + [[nodiscard]] WorkerMasterPtr get_master() const override { return BendersSequential::get_master(); }; @@ -69,20 +48,15 @@ class BendersSequentialDouble : public BendersSequential { void UpdateTrace() override{}; void post_run_actions() const override{}; void SaveCurrentBendersData() override{}; - void reset_master(WorkerMaster *worker_master) override { - WorkerMasterPtr var; - var.reset(worker_master); - BendersBase::reset_master(new FakeWorkerMaster(var)); - }; void free() override{}; void InitializeProblems() override { MatchProblemToId(); auto solver_log_manager = SolverLogManager(solver_log_file()); - reset_master(new WorkerMaster(master_variable_map_, get_master_path(), + reset_master(master_variable_map_, get_master_path(), get_solver_name(), get_log_level(), _data.nsubproblem, solver_log_manager, - IsResumeMode(), _logger)); + IsResumeMode(), _logger); for (const auto &problem : coupling_map_) { const auto subProblemFilePath = GetSubproblemPath(problem.first); AddSubproblem(problem);