Skip to content

Commit

Permalink
Fix sonar issues (#819)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
JasonMarechal25 authored May 21, 2024
1 parent 92040c2 commit 2033c5c
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 49 deletions.
8 changes: 2 additions & 6 deletions src/cpp/benders/benders_core/BendersBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -720,10 +720,6 @@ std::map<std::string, int> 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;
Expand Down Expand Up @@ -915,10 +911,10 @@ void BendersBase::MasterAddRows(
row_names);
}
void BendersBase::ResetMasterFromLastIteration() {
reset_master(new WorkerMaster(master_variable_map_, LastMasterPath(),
reset_master<WorkerMaster>(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_; }

Expand Down
12 changes: 7 additions & 5 deletions src/cpp/benders/benders_core/OuterLoopInputDataReader.cpp
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
#include "OuterLoopInputDataReader.h"

#include <utility>

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
Expand Down Expand Up @@ -119,7 +121,7 @@ namespace YAML {
template <>
struct convert<OuterLoopSingleInputData> {
static Node encode(const OuterLoopSingleInputData &rhs) {
//
return {};
}

static bool decode(const Node &pattern, OuterLoopSingleInputData &rhs) {
Expand Down Expand Up @@ -152,7 +154,7 @@ struct convert<OuterLoopSingleInputData> {
template <>
struct convert<OuterLoopInputData> {
static Node encode(const OuterLoopInputData &rhs) {
//
return {};
}

static void DecodePatterns(const Node &patterns, OuterLoopInputData &rhs) {
Expand Down
8 changes: 6 additions & 2 deletions src/cpp/benders/benders_core/include/BendersBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename T, typename... Args>
void reset_master(Args &&...args) {
_master = std::make_shared<T>(std::forward<Args>(args)...);
master_is_empty_ = false;
}
void free_master();
void free_subproblems();
void AddSubproblem(const std::pair<std::string, VariableMap> &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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/cpp/benders/benders_mpi/BendersMPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<WorkerMaster>(master_variable_map_, get_master_path(),
get_solver_name(), get_log_level(),
_data.nsubproblem, solver_log_manager_,
IsResumeMode(), _logger));
IsResumeMode(), _logger);
}
}
/*!
Expand Down
4 changes: 2 additions & 2 deletions src/cpp/benders/benders_mpi/include/BendersMPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ class BendersMpi : public BendersBase {
std::shared_ptr<MathLoggerDriver> 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;
Expand Down
4 changes: 2 additions & 2 deletions src/cpp/benders/benders_sequential/BendersSequential.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ BendersSequential::BendersSequential(
void BendersSequential::InitializeProblems() {
MatchProblemToId();

reset_master(new WorkerMaster(master_variable_map_, get_master_path(),
reset_master<WorkerMaster>(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);

Expand Down
32 changes: 3 additions & 29 deletions tests/cpp/benders/benders_sequential_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> 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;
Expand Down Expand Up @@ -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();
};

Expand All @@ -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<WorkerMaster>(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);
Expand Down

0 comments on commit 2033c5c

Please sign in to comment.