Legacy ortools behind API - user story 3.1 #2455
Conversation
guilpier-code
changed the title
…lver prevents solver from being shared among weeks
src/solver/modeler/ortoolsImpl/include/antares/solver/modeler/ortoolsImpl/linearProblem.h
Show resolved
Hide resolved
guilpier-code
requested review from
flomnes,
payetvin,
JasonMarechal25 and
pet-mit
and removed request for
payetvin
| auto ortoolsProblem = std::make_unique<OrtoolsLinearProblem>(Probleme.isMIP(), options.ortoolsSolver); | ||
| auto legacyOrtoolsFiller = std::make_unique<LegacyOrtoolsFiller>(ortoolsProblem->MPSolver(), &Probleme); | ||
| std::vector<LinearProblemFiller*> fillersCollection = {legacyOrtoolsFiller.get()}; | ||
| LinearProblemData LP_Data; | ||
| LinearProblemBuilder linearProblemBuilder(fillersCollection); | ||
|
|
||
| linearProblemBuilder.build(*ortoolsProblem, LP_Data); | ||
| auto MPproblem = std::shared_ptr<MPSolver>(ortoolsProblem->MPSolver()); |
There was a problem hiding this comment.
This block appears twice in this file.
It is meant to create a ortools MPSolver.
It's a kind of code duplication.
This duplication will be removed in a further PR
src/solver/modeler/ortoolsImpl/include/antares/solver/modeler/ortoolsImpl/linearProblem.h
Show resolved
Hide resolved
src/solver/modeler/api/include/antares/solver/modeler/api/fillers/LegacyOrtoolsFiller.h
Outdated
Show resolved
Hide resolved
src/solver/modeler/api/include/antares/solver/modeler/api/fillers/LegacyOrtoolsFiller.h
Outdated
Show resolved
Hide resolved
Signed-off-by: Peter Mitri <[email protected]>
src/solver/modeler/ortoolsImpl/include/antares/solver/modeler/ortoolsImpl/linearProblem.h
Show resolved
Hide resolved
src/solver/optimisation/include/antares/solver/optimisation/LegacyOrtoolsFiller.h
Outdated
Show resolved
Hide resolved
Co-authored-by: Florian Omnès <[email protected]>
…i-3.1' into feature/legacy-ortools-behind-api-3.1
| * You should have received a copy of the Mozilla Public Licence 2.0 | ||
| * along with Antares_Simulator. If not, see <https://opensource.org/license/mpl-2-0/>. | ||
| */ | ||
|
|
There was a problem hiding this comment.
I can't understand the point of this new class.
It seems like it adds nothing to its parent class.
Is the parent class not enough ?
There was a problem hiding this comment.
For now, this only makes MPSolver public. As MPSolver is only needed in legacy code, this allows us to keep the design of the parent class clean: other users of the parent class must not need access to MPSolver
| #include "antares/solver/utils/named_problem.h" | ||
| #include "antares/solver/utils/ortools_utils.h" | ||
|
|
||
| #include "ortools/linear_solver/linear_solver.h" |
There was a problem hiding this comment.
#include "ortools/linear_solver/linear_solver.h"We don't need that
|
|
||
| #include "antares/solver/modeler/api/linearProblemFiller.h" | ||
| #include "antares/solver/utils/named_problem.h" | ||
| #include "antares/solver/utils/ortools_utils.h" |
There was a problem hiding this comment.
About :
#include "antares/solver/utils/ortools_utils.h"It seems that the only reason for which we have this inclusion is the definition of class Nomenclature.
But this class :
- has nothing to do with ortools or ortools utils
- is only used in current files LegacyFiller.h/.cpp
So we should move this class from ortools_utils.h to LegacyFiller.h, and remove this previously pointed inclusion.
I tested it : code compiles.
And honestly, this class makes so little thing that we can wonder if it's useful, and should not be replaced with simple data structures inside the current filler.
|
In file linearProblemBuilder.cpp (not touched in this PR), there is a not needed inclusion : #include <memory>We should remove it. |
Signed-off-by: Peter Mitri <[email protected]>
Signed-off-by: Peter Mitri <[email protected]>
|
Do not merge with changes on CI yaml files
What we plan to do :
More specifically, we replace the current ortools LP building with a use :
--use-ortools --ortools-solver siriusNotes :
--use-ortools --ortools-solver siriusin the CI. We can spot the expected regressions.