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

Multiple Maximum Generation and Pumping for Hydro (RTEi's -> CR23) #1470

Closed
wants to merge 92 commits into from

Conversation

Milos-RTEi
Copy link
Collaborator

@Milos-RTEi Milos-RTEi commented Jul 31, 2023

This PR is created in order to implement RTEi development on CR23 (Multiple Maximum Generation and Pumping for Hydro).
Spec available at:
https://rteinternational.sharepoint.com/:w:/s/AntaresUser'sClub-Interne/EeDOZ0wxTLJHujcT1I0rMhMBsfOLYbf18_p9_fgWyp1dgQ?e=3uptbu
Test example available at:
https://rteinternational.sharepoint.com/:u:/s/AntaresUser'sClub-Interne/EVa5797c569OjLKHX0hGATAB6ofpwN2nLJvBoAKnchPfcA?e=8hDZBb

TODO:

  • Fix crashed unit & end-to-end test
  • Sonar
  • Unit Tests
  • Refactor-review from RTE
  • Merge Conflicts

src/libs/antares/study/scenario-builder/rules.cpp Outdated Show resolved Hide resolved
src/libs/antares/study/parts/hydro/series.h Outdated Show resolved Hide resolved
src/libs/antares/study/scenario-builder/rules.cpp Outdated Show resolved Hide resolved
src/libs/antares/study/scenario-builder/applyToMatrix.hxx Outdated Show resolved Hide resolved
src/solver/aleatoire/alea_tirage_au_sort_chroniques.cpp Outdated Show resolved Hide resolved
src/solver/hydro/management/daily.cpp Outdated Show resolved Hide resolved
src/solver/hydro/management/management.cpp Outdated Show resolved Hide resolved
src/solver/hydro/management/management.cpp Outdated Show resolved Hide resolved
src/solver/hydro/management/management.cpp Outdated Show resolved Hide resolved
src/solver/hydro/management/management.cpp Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Sep 21, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

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

29.7% 29.7% Coverage
0.6% 0.6% Duplication

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

src/solver/simulation/sim_calcul_economique.cpp Outdated Show resolved Hide resolved
src/solver/simulation/sim_calcul_economique.cpp Outdated Show resolved Hide resolved
src/libs/antares/study/area/scratchpad.cpp Outdated Show resolved Hide resolved
src/solver/simulation/sim_calcul_economique.cpp Outdated Show resolved Hide resolved
src/solver/simulation/sim_structure_donnees.h Outdated Show resolved Hide resolved
src/solver/variable/economy/max-mrg.cpp Outdated Show resolved Hide resolved
src/solver/variable/economy/max-mrg.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@nikolaredstork nikolaredstork left a comment

Choose a reason for hiding this comment

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

Please check also my comments.

@flomnes flomnes added the feature label Oct 6, 2023
src/solver/simulation/timeseries-numbers.cpp Outdated Show resolved Hide resolved
if (nbTimeSeries != 1)
{
area.hydro.series->timeseriesNumbersPowerCredits[0][year] = static_cast<uint32_t>(
(floor(study.runtime->random[seedTimeseriesNumbers].next() * nbTimeSeries)));
Copy link
Contributor

Choose a reason for hiding this comment

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

About study.runtime->random[seedTimeseriesNumbers].next() :
The problem here may be :
when we draw a number, we introduce a shift in the random drawer.
So, what may happen here is that the TS numbers drawn for the next category (here Thermal), we be shifted and be different.
Consequence : you have a study which has only one max power TS (for generation and pumping), like studies are before the current evolution. You resize this number of TS to 10 or 100 (whatever), and you set each TS to a great number or infinity. We can expect that the results of this study will be the same as they once were.
Well they won't, because we introduced a shift in the TS numbers drawing, and for thermal, they are now different.
Is it important ?
It remains to be seen, but it's worth pointing it here.

Copy link
Member

@sylvlecl sylvlecl left a comment

Choose a reason for hiding this comment

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

A first set of commands from my side

src/libs/antares/study/area/list.cpp Outdated Show resolved Hide resolved
src/libs/antares/study/area/list.cpp Show resolved Hide resolved
src/libs/antares/study/area/list.cpp Outdated Show resolved Hide resolved
src/libs/antares/study/fwd.h Show resolved Hide resolved

buffer.clear() << base_folder << SEP << hydro_folder;
ret = datatransfer->LoadFromFolder(*study, buffer, *area_2);
BOOST_CHECK(!ret);
Copy link
Member

Choose a reason for hiding this comment

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

In all tests:
we should check the content of the study, not only the boolean result.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is tested, please look Refactor PR #1648

src/solver/simulation/sim_calcul_economique.cpp Outdated Show resolved Hide resolved
src/solver/simulation/sim_calcul_economique.cpp Outdated Show resolved Hide resolved
* Power Credits comments and strings changed to Max Power

* Changing name convention

* Implicit conversion code smell

* Files name modification

* Renaming matrices from PartHydro and DataTransfer classes

* Renaming matrices from DataSeriesHydro class

* HydroMaxTSReader class

* Refactoring

* New LoadMaxPower function

* bugfix

* Excluding unnecessary data from tests and bugfix

* Code smell

* Daily mean maximum generation/pumping matrices

* Check Power Bounds

* Rewrite comment

* Name convention

* Code smells

* Comments and function name changed

* Lambda function refactor

* Get and Set functions

* Small refactoring

* Name convention

* Ternary operator check index excluded

* Clang Format undone

* Function argument names

* Possible Bug?

* Refactoring lambda function

* Refactoring Unit tests

* Refactoring unit tests

* Unit tests

* Typo and renaming

* Unit test fix and renaming

* hydro reader unit tests refactoring

* Refactoring unit tests help functions

* Excluding saving new files before upgrade

* Renameing files

* Get function instead of class data member

* Refactor hydro max power TS consistency (#1707)

* Refactor max power TS checks : several things (see commit message)

This commit is large, it should be shorter, but the long compilation time forces to gather the devs in one piece.
- Remove the exception raising, not necessary : the gotFatalError (set to true) will trigger an exception anyway.
- class NbTsComparer : is generalized to any time series pair :
   + to isolate it from business context
   + to have a chance to re-use it in another context
- Move both classes NbTsComparer and TsActions outside class DataSeriesHydro, for same reasons as above
- Rename TsActions::handleBothGreaterThanOne (unclear) into something more meaningful
- Consequences of previous changes in tests

* Refactor max power TS checks : several things (see commit message)

TsActions::areaToInvalidate :
- Remove areaToInvalidate from class TsActions (it has nothing to do in here) and make it a free function
- Extract areaToInvalidate calls from TsActions::resizeWhenOneTS, as calling it here not related to TsActions

* Refactor max power TS checks : simplify TsActions::resizeWhenOneTS

* Refactor max power TS checks : remove a useless argument to free function invalidateArea

* Refactor max power TS checks : changing invalidateArea's signature and content

In this context, pointer area (as argument) is necessarily not null, because it is used way before we come to this point.
So :
- We make area a reference
- We stop testing the area address (we known it's not null)
- We remove the dead code (when area address is null)

* Refactor max power TS checks : several things (see commit message)

- Move class NumberComparison to separate source files
- Remove return type to function postProcessMaxPowerTS : no more boolean returned, but void.
  Returning boolean is for loading tasks, here we check and possibly change already loaded data.
  We adapt tests, as a consequence
- Removing class TsActions, not useful
-

* Refactor max power TS checks : add forgotten files + renaming

* Refactor max power TS checks : remove case where both TS matrices (pump + gen) have null width, impossible

* Refactor max power TS checks : just removing useless comments

* Refactor max power TS checks : a bot of cleaning

* Refactor max power TS checks : internalize count of generation time series (ror, storage, mingen)

We should not be able to change a TS count without resizing the TS.
This was allowed when because this count was a public data member.
So we make it a private data member, and change it only when resizing the associated data.

* Refactor max power TS checks : renaming

* Refactor max power TS checks : renaming

* Refactor max power TS checks : adapt CI Window workflow to new runner (about python)

* Refactor max power TS checks : define and use of function EqualizeTSsize(...)

* Refactor max power TS checks : rearrange DataSeriesHydro::loadFromFolder, but no major change

* Refactor max power TS checks : remove the case where TS (ror, storage, mingen) have a zero size. Impossible case.

* Refactor max power TS checks : fix bugs on function EqualizeTSsize

* Refactor max power TS checks :  use maxPowerTScount() whenever possible

* Refactor max power TS checks : removing the setter for max power TS number

* Refactor max power TS checks : fix a bug due to removing the setter for max power TS number

* Refactor max power TS checks : gather loading and equalizing hydro mingen TS

* Refactor max power TS checks : split load of ROR, STORAGE and MINGEN

* Refactor max power TS checks : loading Max Power TS : using a generic loading function

* Refactor max power TS checks : fix a bug on loading inflows (height ont correct)

* Refactor max power TS checks : correction after (small) merge

* Refactor max power TS checks : fixing bug on loading inflows was wrong. Fixing it for real now.

* Refactor max power TS checks : remove useless checkMinGenTsNumber

* Refactor max power TS checks : cleaning on the very ugly setHydroModulability (too many args, body too big)

* Refactor max power TS checks : correct derated mode for hydro

* Refactor max power TS checks : some more corrections due to review

* Refactor max power TS checks : some more corrections after review

* Refactor max power TS checks : fixing errors introduced by earlier corrections

* Refactor max power TS checks : more corrections after review

* Refactor max power TS checks : more corrections due to tight review :-)

* Fixing sonar bug

* Code smells

* Use init-statement inside the if statement for hydroSeries ptr

* More code smells

---------

Co-authored-by: NikolaIlic <[email protected]>

* Bugfix

* Code smells

---------

Co-authored-by: guilpier-code <[email protected]>
@flomnes
Copy link
Member

flomnes commented Jan 18, 2024

PR with fewer conflicts #1723

@flomnes flomnes closed this Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Technical design
Development

Successfully merging this pull request may close these issues.

5 participants