Skip to content
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

Move data validation to specialized functions [ANT-1213] #2149

Merged
merged 24 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libs/antares/study/area/list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,7 @@ static bool AreaListLoadFromFolderSingleArea(Study& study,
// if changes are required, please update reloadXCastData()
buffer.clear() << study.folderInput << SEP << "hydro" << SEP << "prepro";
ret = area.hydro.prepro->loadFromFolder(study, area.id, buffer.c_str()) && ret;
ret = area.hydro.prepro->validate(area.id) && ret;
}

auto* hydroSeries = area.hydro.series;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ class PreproHydro
** \param folder The source folder (ex: `input/hydro/prepro`)
** \return A non-zero value if the operation succeeded, 0 otherwise
*/
bool loadFromFolder(Study& s, const AreaName& areaID, const char folder[]);
bool loadFromFolder(Study& s, const AreaName& areaID, const std::string& folder);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible (or usefull) to use std::filesystem::path for folder ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be done in another PR


bool validate(const std::string& areaID);
/*!
** \brief Save hydro settings for the prepro into a folder
**
Expand Down
33 changes: 19 additions & 14 deletions src/libs/antares/study/parts/hydro/hydromaxtimeseriesreader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ HydroMaxTimeSeriesReader::HydroMaxTimeSeriesReader(PartHydro& hydro,
dailyMaxPumpAndGen.reset(4U, DAYS_PER_YEAR, true);
}

static bool checkPower(const Matrix<>& dailyMaxPumpAndGen, const std::string& areaName)
{
for (uint i = 0; i < 4U; ++i)
{
auto& col = dailyMaxPumpAndGen[i];
for (uint day = 0; day < DAYS_PER_YEAR; ++day)
{
if (col[day] < 0. || (i % 2U /*column hours*/ && col[day] > 24.))
{
logs.error() << areaName << ": invalid power or energy value";
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carefull
With the early return you are returning after the first error. Previous behaviour is to check all values and log all errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!errorPower avoided going into this scope after the first error, I simplified while keeping the same behavior

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No you don't keep the same behavior. The user will only see the 1st error, not the following ones if any.

}
}
}

return true;
}

bool HydroMaxTimeSeriesReader::loadDailyMaxPowersAndEnergies(const AnyString& folder,
bool usedBySolver)
{
Expand Down Expand Up @@ -90,21 +108,8 @@ bool HydroMaxTimeSeriesReader::loadDailyMaxPowersAndEnergies(const AnyString& fo
Matrix<>::optFixedSize,
&fileContent)
&& ret;
ret = checkPower(dailyMaxPumpAndGen, areaName_) && ret;

bool errorPowers = false;
for (uint i = 0; i < 4U; ++i)
{
auto& col = dailyMaxPumpAndGen[i];
for (uint day = 0; day < DAYS_PER_YEAR; ++day)
{
if (!errorPowers && (col[day] < 0. || (i % 2U /*column hours*/ && col[day] > 24.)))
{
logs.error() << areaName_ << ": invalid power or energy value";
errorPowers = true;
ret = false;
}
}
}
}
return ret;
}
Expand Down
105 changes: 48 additions & 57 deletions src/libs/antares/study/parts/hydro/prepro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,10 @@ bool PreproHydro::saveToFolder(const AreaName& areaID, const char* folder)
return false;
}

bool PreproHydro::loadFromFolder(Study& s, const AreaName& areaID, const char* folder)
bool PreproHydro::loadFromFolder(Study& s, const AreaName& areaID, const std::string& folder)
{
/* Asserts */
assert(folder);
assert('\0' != *folder);
assert(!folder.empty());
payetvin marked this conversation as resolved.
Show resolved Hide resolved

enum
{
Expand All @@ -161,6 +160,17 @@ bool PreproHydro::loadFromFolder(Study& s, const AreaName& areaID, const char* f

buffer.clear() << folder << SEP << areaID << SEP << "prepro.ini";
bool ret = PreproHydroLoadSettings(this, buffer);

buffer.clear() << folder << SEP << areaID << SEP << "energy.txt";
ret = data.loadFromCSVFile(buffer, hydroPreproMax, 12, mtrxOption, &s.dataBuffer) && ret;
payetvin marked this conversation as resolved.
Show resolved Hide resolved

return ret;
}

bool PreproHydro::validate(const std::string& areaID)
{
bool ret = true;

if (intermonthlyCorrelation < 0. || intermonthlyCorrelation > 1.)
{
logs.error() << "Hydro: Prepro: `" << areaID
Expand All @@ -175,77 +185,58 @@ bool PreproHydro::loadFromFolder(Study& s, const AreaName& areaID, const char* f
}
}

buffer.clear() << folder << SEP << areaID << SEP << "energy.txt";
ret = data.loadFromCSVFile(buffer, hydroPreproMax, 12, mtrxOption, &s.dataBuffer) && ret;

if (JIT::enabled)
{
return ret;
}

// Checks
const auto& col = data[powerOverWater];
for (unsigned i = 0; i != data.height; ++i)
{
auto& col = data[powerOverWater];
for (uint i = 0; i != data.height; ++i)
const double d = col[i];
if (d < 0. || d > 1.)
{
const double d = col[i];
if (d < 0. || d > 1.)
{
logs.error() << "Hydro: Prepro: " << areaID
<< ": invalid value for ROR (line: " << (i + 1) << ")";
}
logs.error() << "Hydro: Prepro: " << areaID
<< ": invalid value for ROR (line: " << (i + 1) << ")";
}
}

{
auto& colMin = data[minimumEnergy];
auto& colMax = data[maximumEnergy];
const auto& colMin = data[minimumEnergy];
const auto& colMax = data[maximumEnergy];

for (uint i = 0; i != data.height; ++i)
for (unsigned i = 0; i != data.height; ++i)
{
if (colMin[i] < 0.)
{
if (colMin[i] < 0.)
{
ret = false;
logs.error() << "Hydro: Prepro: `" << areaID
<< "`: minimum energy: At least one value is negative (line: "
<< (i + 1) << ')';
continue;
}
if (colMin[i] > colMax[i])
{
ret = false;
logs.error() << "Hydro: Prepro: `" << areaID
<< "`: the minimum energy must be less than the maximum energy (line: "
<< (i + 1) << ')';
}
ret = false;
logs.error() << "Hydro: Prepro: `" << areaID
<< "`: minimum energy: At least one value is negative (line: "
<< (i + 1) << ')';
continue;
}
if (colMin[i] > colMax[i])
{
ret = false;
logs.error() << "Hydro: Prepro: `" << areaID
<< "`: the minimum energy must be less than the maximum energy (line: "
<< (i + 1) << ')';
}
}

const auto& colExp = data[expectation];
for (unsigned i = 0; i != data.height; i++)
{
auto& colExp = data[expectation];

for (uint i = 0; i != data.height; i++)
if (colExp[i] < 0.)
{
if (colExp[i] < 0.)
{
ret = false;
logs.error() << "Hydro: Prepro: `" << areaID
<< "`: invalid value for expectation (line: " << (i + 1) << ")";
}
ret = false;
logs.error() << "Hydro: Prepro: `" << areaID
<< "`: invalid value for expectation (line: " << (i + 1) << ")";
}
}

const auto& colStdDev = data[stdDeviation];
for (unsigned i = 0; i != data.height; i++)
{
auto& colStdDev = data[stdDeviation];

for (uint i = 0; i != data.height; i++)
if (colStdDev[i] < 0.)
{
if (colStdDev[i] < 0.)
{
ret = false;
logs.error() << "Hydro: Prepro: `" << areaID
<< "`: invalid value for standard deviation (line: " << (i + 1) << ")";
}
ret = false;
logs.error() << "Hydro: Prepro: `" << areaID
<< "`: invalid value for standard deviation (line: " << (i + 1) << ")";
}
}

Expand Down
Loading