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.

Sign up for GitHub

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

Update Final reservoir level implementation - cr25 #1564

Merged
merged 8 commits into from
Sep 28, 2023
Merged

Update Final reservoir level implementation - cr25 #1564

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4ffcfd0
fix - isApplicable
Milos-RTEi Aug 17, 2023
e191e5a
add logs if fin-lvl not applicable
Milos-RTEi Aug 17, 2023
d38d482
fix - only check active years
Milos-RTEi Sep 8, 2023
0c37370
fix - failed unit test yearFilter
Milos-RTEi Sep 8, 2023
41dd16d
refactor unit test
Milos-RTEi Sep 13, 2023
94648ef
improve info log + refactor
Milos-RTEi Sep 13, 2023
0cda6fe
Merge branch 'fix/hydro-final-levels-cr25--try-fixes' into fix/hydro-…
guilpier-code Sep 14, 2023
950a0a2
CR-25 - add-info : correction after merge from develop
guilpier-code Sep 14, 2023
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
15 changes: 14 additions & 1 deletion src/libs/antares/study/parts/hydro/finallevelinflowsmodifyer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ FinalLevelInflowsModifier::FinalLevelInflowsModifier(const PartHydro& hydro,
bool FinalLevelInflowsModifier::CheckInfeasibility(uint year)
{
ComputeDelta(year);
logInfoFinLvlNotApplicable(year);

if (!isActive())
return true;
Expand Down Expand Up @@ -147,6 +148,18 @@ bool FinalLevelInflowsModifier::isActive()
isValidLevel(initialReservoirLevel_);
}

// 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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

@Milos-RTEi Milos-RTEi Sep 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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.

{
if (isValidLevel(finalReservoirLevel_)
&& (!hydro_.reservoirManagement || hydro_.useWaterValue
|| !isValidLevel(initialReservoirLevel_)))
logs.info() << "Final reservoir level not applicable! Year:" << year + 1
<< ", Area:" << areaName_;
Copy link
Member

Choose a reason for hiding this comment

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

Please tell the user why the final level is not applicable, so that they can change their input data accordingly.

}

bool FinalLevelInflowsModifier::makeChecks(uint year)
{
// Simulation must end on day 365 and reservoir level must be initiated in January
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

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

Both versions are correct, though using empty makes it a bit more clear

Suggested change
return isApplicable_.size() && isApplicable_.at(year);
return !isApplicable_.empty() && isApplicable_.at(year);

}

} // namespace Antares::Data
1 change: 1 addition & 0 deletions src/libs/antares/study/parts/hydro/finallevelinflowsmodifyer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class FinalLevelInflowsModifier

private:
bool isActive();
void logInfoFinLvlNotApplicable(uint year);
void ComputeDelta(uint year);
bool makeChecks(uint year);
void storeDeltaLevels(uint year);
Expand Down