From d23369185ef4851a2b9c63345aab73f2b0cddea1 Mon Sep 17 00:00:00 2001 From: payetvin <113102157+payetvin@users.noreply.github.com> Date: Fri, 21 Jun 2024 13:36:38 +0200 Subject: [PATCH] Separation of loading and validation for parameters [ANT-1213] (#2177) --- .../study/include/antares/study/parameters.h | 5 +- src/libs/antares/study/load.cpp | 8 +- src/libs/antares/study/parameters.cpp | 153 ++++++++---------- .../study/parameters/parameters-tests.cpp | 4 + 4 files changed, 78 insertions(+), 92 deletions(-) diff --git a/src/libs/antares/study/include/antares/study/parameters.h b/src/libs/antares/study/include/antares/study/parameters.h index cb69fb4326..791e20f865 100644 --- a/src/libs/antares/study/include/antares/study/parameters.h +++ b/src/libs/antares/study/include/antares/study/parameters.h @@ -137,6 +137,8 @@ class Parameters final */ void fixBadValues(); + void validateOptions(const StudyLoadOptions&); + /*! ** \brief Try to detect then fix refresh intervals */ @@ -503,8 +505,7 @@ class Parameters final private: //! Load data from an INI file bool loadFromINI(const IniFile& ini, - const StudyVersion& version, - const StudyLoadOptions& options); + const StudyVersion& version); void resetPlayedYears(uint nbOfYears); diff --git a/src/libs/antares/study/load.cpp b/src/libs/antares/study/load.cpp index 00214717e8..4555e106bd 100644 --- a/src/libs/antares/study/load.cpp +++ b/src/libs/antares/study/load.cpp @@ -83,7 +83,13 @@ bool Study::internalLoadIni(const String& path, const StudyLoadOptions& options) } // Load the general data buffer.clear() << folderSettings << SEP << "generaldata.ini"; - if (!parameters.loadFromFile(buffer, header.version, options)) + bool errorWhileLoading = !parameters.loadFromFile(buffer, header.version, options); + + parameters.validateOptions(options); + + parameters.fixBadValues(); + + if (errorWhileLoading) { if (options.loadOnlyNeeded) { diff --git a/src/libs/antares/study/parameters.cpp b/src/libs/antares/study/parameters.cpp index 5c0cdac583..7337b9c8b0 100644 --- a/src/libs/antares/study/parameters.cpp +++ b/src/libs/antares/study/parameters.cpp @@ -566,43 +566,11 @@ static bool SGDIntLoadFamily_General(Parameters& d, if (key == "simulation.start") { - uint day; - if (not value.to(day)) - { - return false; - } - if (day == 0) - { - day = 1; - } - else - { - if (day > 365) - { - day = 365; - } - --day; - } - d.simulationDays.first = day; - return true; + return value.to(d.simulationDays.first); } if (key == "simulation.end") { - uint day; - if (not value.to(day)) - { - return false; - } - if (day == 0) - { - day = 1; - } - else if (day > 365) - { - day = 365; - } - d.simulationDays.end = day; // not included - return true; + return value.to(d.simulationDays.end); } if (key == "thematic-trimming") @@ -1158,8 +1126,7 @@ bool firstKeyLetterIsValid(const String& name) } bool Parameters::loadFromINI(const IniFile& ini, - const StudyVersion& version, - const StudyLoadOptions& options) + const StudyVersion& version) { // Reset inner data reset(); @@ -1229,62 +1196,10 @@ bool Parameters::loadFromINI(const IniFile& ini, } } - // forcing value - if (options.nbYears != 0) - { - if (options.nbYears > nbYears) - { - // The variable `yearsFilter` must be enlarged - yearsFilter.resize(options.nbYears, false); - } - nbYears = options.nbYears; - - // Resize years weight (add or remove item) - if (yearsWeight.size() != nbYears) - { - yearsWeight.resize(nbYears, 1.f); - } - } - - // Simulation mode - // ... Enforcing simulation mode - if (options.forceMode != SimulationMode::Unknown) - { - mode = options.forceMode; - logs.info() << " forcing the simulation mode " << SimulationModeToCString(mode); - } - else - { - logs.info() << " simulation mode: " << SimulationModeToCString(mode); - } - - if (options.forceDerated) - { - derated = true; - } - - namedProblems = options.namedProblems; - - handleOptimizationOptions(options); - - // Attempt to fix bad values if any - fixBadValues(); - fixRefreshIntervals(); fixGenRefreshForNTC(); - // Specific action before launching a simulation - if (options.usedByTheSolver) - { - prepareForSimulation(options); - } - - if (options.mpsToExport || options.namedProblems) - { - this->include.exportMPS = mpsExportStatus::EXPORT_BOTH_OPTIMS; - } - // We currently always returns true to not block any loading process // Anyway we already have reported all problems return true; @@ -1375,6 +1290,66 @@ void Parameters::fixBadValues() { nbTimeSeriesSolar = 1; } + + if (simulationDays.first == 0) + simulationDays.first = 1; + else + { + simulationDays.first = std::clamp(simulationDays.first, 1u, 365u); + --simulationDays.first; // value between 0 and 364 for edge cases + } + + simulationDays.end = std::clamp(simulationDays.end, 1u, 365u); +} + +void Parameters::validateOptions(const StudyLoadOptions& options) +{ + if (options.forceDerated) + { + derated = true; + } + // forcing value + if (options.nbYears != 0) + { + if (options.nbYears > nbYears) + { + // The variable `yearsFilter` must be enlarged + yearsFilter.resize(options.nbYears, false); + } + nbYears = options.nbYears; + + // Resize years weight (add or remove item) + if (yearsWeight.size() != nbYears) + { + yearsWeight.resize(nbYears, 1.f); + } + } + + // Simulation mode + // ... Enforcing simulation mode + if (options.forceMode != SimulationMode::Unknown) + { + mode = options.forceMode; + logs.info() << " forcing the simulation mode " << SimulationModeToCString(mode); + } + else + { + logs.info() << " simulation mode: " << SimulationModeToCString(mode); + } + // Specific action before launching a simulation + if (options.usedByTheSolver) + { + prepareForSimulation(options); + } + + if (options.mpsToExport || options.namedProblems) + { + this->include.exportMPS = mpsExportStatus::EXPORT_BOTH_OPTIMS; + } + + namedProblems = options.namedProblems; + + handleOptimizationOptions(options); } uint64_t Parameters::memoryUsage() const @@ -1990,7 +1965,7 @@ bool Parameters::loadFromFile(const AnyString& filename, IniFile ini; if (ini.open(filename)) { - return loadFromINI(ini, version, options); + return loadFromINI(ini, version); } // Error otherwise diff --git a/src/tests/src/libs/antares/study/parameters/parameters-tests.cpp b/src/tests/src/libs/antares/study/parameters/parameters-tests.cpp index 14ee6a4435..07c8e7fa1d 100644 --- a/src/tests/src/libs/antares/study/parameters/parameters-tests.cpp +++ b/src/tests/src/libs/antares/study/parameters/parameters-tests.cpp @@ -70,6 +70,8 @@ BOOST_FIXTURE_TEST_CASE(loadValid, Fixture) writeValidFile(); p.loadFromFile(path.string(), version, options); + p.validateOptions(options); + p.fixBadValues(); BOOST_CHECK_EQUAL(p.nbYears, 5); BOOST_CHECK_EQUAL(p.seed[seedTsGenThermal], 5489); @@ -93,6 +95,8 @@ BOOST_FIXTURE_TEST_CASE(invalidValues, Fixture) { writeInvalidFile(); BOOST_CHECK(p.loadFromFile(path.string(), version, options)); + p.validateOptions(options); + p.fixBadValues(); BOOST_CHECK_EQUAL(p.nbYears, 1); BOOST_CHECK_EQUAL(p.useCustomScenario, 0);