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

Hydro final lvl (CR 25) to merge in develop [ANT-1084] #1521

Open
wants to merge 584 commits into
base: develop
Choose a base branch
from

Conversation

guilpier-code
Copy link
Contributor

@guilpier-code guilpier-code commented Aug 8, 2023

This PR attempts some code enhancements on branch feature/final-reservoir-level-cr25.
Therefore, this PR's associated branch (fix/hydro-final-levels-cr25--try-fixes) is based on feature/final-reservoir-level-cr25.

  • in class FinalLevelInflowsModifier,
    • try to reduce the number of data members,
    • making data members private whenever it's possible,
    • define a clear interface with a smallest number of public functions
    • and so make the more methods private as possible
  • Unit tests : clarify their intention and reduce their size
  • caution : could not remove yet the initialize method. This method should be merged with the constructor, but when we call the constructor, all input pieces of data are not available. that's why initialize exists. We should try to make these pieces of data available, for instance by changing the order of data loading

For any purpose, here are doc resources :

  • specs here
  • a discussion on that matter here

CAUTION :

Some cleaning and corrections were introduced in a new pull request (see this PR).
This new PR was merged into this one.
This PR contains a test under the form of a study.
This study should be downloaded and added to the right test repo.


Left to to

  • Merge from branch develop and remove conflicts
  • Check that all tests exist (unit test for raising failures and study case for fine working)
  • what else ?

Milos-RTEi and others added 30 commits August 24, 2022 11:11
CSR comments implementation update
Copy link

sonarcloud bot commented Nov 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 20 Code Smells

73.9% 73.9% Coverage
0.0% 0.0% Duplication

Copy link

sonarcloud bot commented Jan 18, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

20 New issues
0 Security Hotspots
73.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Apr 11, 2024

@flomnes flomnes modified the milestones: v8.9.0, 9.2 May 2, 2024
See comments to know more about this PR intentions.

Furthermore, a study testing final levels can be found here below.
Please read this study's user's notes for the test goals and how to
fulfill them.


[final-levels.zip](https://github.com/AntaresSimulatorTeam/Antares_Simulator/files/15126508/final-levels.zip)
@flomnes flomnes changed the title Hydro final lvl (CR 25) to merge in develop Hydro final lvl (CR 25) to merge in develop [ANT-1084] Jun 10, 2024
@flomnes flomnes modified the milestones: 9.2, Sprint 5 Jun 10, 2024
@flomnes
Copy link
Member

flomnes commented Jun 17, 2024

Final reservoir level not applicable should be a warning, not an info

Comment on lines 123 to 125
hydroLevelsData hydroInitialLevels = hydroLevelsData("hl,", initLevelApply);
//! hydro final levels
hydroLevelsData hydroFinalLevels = hydroLevelsData("hfl,", finalLevelApply);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
hydroLevelsData hydroInitialLevels = hydroLevelsData("hl,", initLevelApply);
//! hydro final levels
hydroLevelsData hydroFinalLevels = hydroLevelsData("hfl,", finalLevelApply);
hydroLevelsData hydroInitialLevels{"hl,", initLevelApply};
//! hydro final levels
hydroLevelsData hydroFinalLevels{"hfl,", finalLevelApply};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

src/antares-deps Outdated
Copy link
Member

Choose a reason for hiding this comment

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

revert this change

if (hydro_.reservoirManagement && !hydro_.useWaterValue)
return true;

logs.info() << "Final reservoir level not applicable! Year:" << year_ + 1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logs.info() << "Final reservoir level not applicable! Year:" << year_ + 1
logs.warning() << "Final reservoir level not applicable! Year:" << year_ + 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

guilpier-code and others added 3 commits June 18, 2024 11:02
Recall : **Hydro allocation** is done at the beginning of each MC year,
and ends with a quantity of hydro energy for each day of the year. These
daily quantities are to be used as daily hydro generations in the
optimization problem.

The **hydro debug** functionality allows to print (if asked by user) on
disk a **report** about **hydro allocation results**.

This hydro allocation report is broken (a crash comes up). It was broken
since January 2023 (a commit can be supplied if needed).

This PR to fix this crash.
Copy link

sonarcloud bot commented Jun 19, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In dev
Development

Successfully merging this pull request may close these issues.

None yet

4 participants