Update Final reservoir level implementation - cr25 #1564
Conversation
| // if the user specifies the final reservoir level, but does not specifies initial reservoir level | ||
| // or uses wrong hydro options | ||
| // we should inform the user that the final reservoir level wont be reached | ||
| void FinalLevelInflowsModifier::logInfoFinLvlNotApplicable(uint year) |
There was a problem hiding this comment.
To me, there is some inconsistencies here :
- We log that the problem is not applicable, ok, but we don't skip or stop anything because of it. If the problem is not applicable, normally we should prevent something from happening, right ?
- Applicable is already taken by another property (see method isApplicable). We should distinguish the different form of applicability, or it starts to be unclear
There was a problem hiding this comment.
These info logs are as Florian suggested for crazy users.
For example, final_level is set to 55% but initila_level is left at rand.
In this case, we log out that for the specified MC-year and area, the final reservoir level won't be implemented (will be skipped). But we continue on implementation for the next year/area.
Logic won't be implemented for this year/area because "isApplicable_" is going to be false for that year/area.
This behavior should be distinguished from the one when some of the pre-checks fail and we break the simulation immediately.
So we have situations where we exit Antares asap, or we skip the year/area and throw an info log.
| @@ -169,7 +182,7 @@ bool FinalLevelInflowsModifier::isApplicable(uint year) | |||
| { | |||
| // If isApplicable_.size() == 0, then instance was not properly initialized | |||
| // and is not applicable. | |||
| return isActive() && isApplicable_.size() && isApplicable_.at(year); | |||
| return isApplicable_.size() && isApplicable_.at(year); | |||
There was a problem hiding this comment.
Both versions are correct, though using empty makes it a bit more clear
| return isApplicable_.size() && isApplicable_.at(year); | |
| return !isApplicable_.empty() && isApplicable_.at(year); |
| if (study.parameters.yearsFilter.at(year)) | ||
| CheckFinalReservoirLevelsForYear(study, year); |
There was a problem hiding this comment.
Checking only active years is good I think
| std::vector<bool> yearFilter(2, true); | ||
| study->parameters.yearsFilter = yearFilter; |
There was a problem hiding this comment.
Slightly simpler
| std::vector<bool> yearFilter(2, true); | |
| study->parameters.yearsFilter = yearFilter; | |
| study->parameters.yearsFilter.assign(2, true); |
| if (isValidLevel(finalReservoirLevel_) | ||
| && (!hydro_.reservoirManagement || hydro_.useWaterValue | ||
| || !isValidLevel(initialReservoirLevel_))) | ||
| logs.info() << "Final reservoir level not applicable! Year:" << year + 1 | ||
| << ", Area:" << areaName_; |
There was a problem hiding this comment.
Please tell the user why the final level is not applicable, so that they can change their input data accordingly.
…final-level-cr25-add-info # Conflicts: # src/libs/antares/study/parts/hydro/finallevelinflowsmodifyer.cpp # src/libs/antares/study/parts/hydro/finallevelinflowsmodifyer.h
|
Kudos, SonarCloud Quality Gate passed! |
flomnes
merged commit ddd1fdc
into
fix/hydro-final-levels-cr25--try-fixes
This PR solves an issue and adds info logs on branch [fix/hydro-final-levels-cr25--try-fixes].