-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove duplication in simulation run #2274
Conversation
@@ -95,6 +96,9 @@ class Application final: public Yuni::IEventObserver<Application, Yuni::Policy:: | |||
void runSimulationInAdequacyMode(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These methods are about to be removed
void runSimulationInAdequacyMode();
void runSimulationInEconomicMode();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now removed
@@ -104,8 +105,6 @@ SimulationResults APIInternal::execute() const | |||
// Importing Time-Series if asked | |||
study_->importTimeseriesIntoInput(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
study_->importTimeseriesIntoInput();
Do we need this in API ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not
Quality Gate passedIssues Measures |
063ae29
to
1694e4c
Compare
…lClusters automatically when running a simulation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor remarks
src/solver/simulation/include/antares/solver/simulation/simulation-runner.h
Outdated
Show resolved
Hide resolved
@@ -104,8 +105,6 @@ SimulationResults APIInternal::execute() const | |||
// Importing Time-Series if asked | |||
study_->importTimeseriesIntoInput(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not
#include "antares/solver/simulation/adequacy.h" | ||
#include "antares/solver/simulation/economy.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These headers are not required here, please move them to simulation-runner.cpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, they will have to be moved to simulation-runner.cpp, where they are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Quality Gate passedIssues Measures |
All comments were taken into account
This work just started...
Situation :
There are code duplications :
Problem :
Difficult to remove the first duplication without removing the second one.
Purpose :
Remove them both.