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

Fix/global var hydro modulable #1614

Merged
merged 6 commits into from
Sep 18, 2023

Conversation

guilpier-code
Copy link
Contributor

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

Before the current pull request, type VAL_GEN_PAR_PAYS contained only data members related to hydro, and even hydro made available by hydro ventilation.

  • Moving VAL_GEN_PAR_PAYS valeursGenereesParPays from class ISimulation into class HydroManagement, and renaming it ALL_HYDRO_VENTILATION_RESULTS ventilationResults.
  • Cleaning the class HydroManagement :
    • Removing dependency to Study, replaced with dependencies to AreaList, Parameters, Calendar, ...
    • Renaming operator () (...) into makeVentilation(...), much more clear

@guilpier-code guilpier-code added the enhancement New feature or request label Sep 8, 2023
@guilpier-code guilpier-code self-assigned this Sep 8, 2023
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Sep 8, 2023
@guilpier-code guilpier-code marked this pull request as ready for review September 11, 2023 07:47
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

0.0% 0.0% Coverage
0.7% 0.7% Duplication

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

Comment on lines +115 to +118
void makeVentilation(double* randomReservoirLevel,
Solver::Variable::State& state,
uint y,
uint numSpace);
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
void makeVentilation(double* randomReservoirLevel,
Solver::Variable::State& state,
uint y,
uint numSpace);
ALL_HYDRO_VENTILATION_RESULTS makeVentilation(double* randomReservoirLevel,
Solver::Variable::State& state,
uint y,
uint numSpace);

One single method instead of two.

Copy link
Contributor Author

@guilpier-code guilpier-code Sep 18, 2023

Choose a reason for hiding this comment

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

ok. So this means that we could simplify the caller too :

// solver.hxx
yearFailed[y] = !simulation_->year(...,
                                   ...,
                                   simulation_->hydroManagement.makeVentilation(/* some args */));

As a consequence, we would skip calls the timer / add duration, unless we put these calls in the HydroManagement :: makeVentilation.
What do you recommend ?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep the timer, but have a single function.

// Timer ON
auto hydroVentilation = simulation_->hydroManagement.makeVentilation(/* args */);
// Timer OFF
yearFailed[y] = !simulation_->year(...,
                                   ...,
                                   hydroVentilation);

src/solver/simulation/economy.h Outdated Show resolved Hide resolved
@flomnes flomnes merged commit 2faa11f into fix/remove-global-vars-2 Sep 18, 2023
@flomnes flomnes deleted the fix/global-var-hydro-modulable branch September 18, 2023 14:50
flomnes added a commit that referenced this pull request Sep 19, 2023
* [skip ci] Remove global vars : a bit of cleaning

* Remove global vars : valeursGenereesParPays's allocation moved in a constructor + simplifying runtime

* Remove global vars : unnecessary initialization of HydrauliqueModulableQuotidien (already done in simulation)

* Remove global vars : remove unnecessary function, as it only calls one other function

* Remove global vars : back to previous state, before wrong correction

* Remove global vars : a bit of clarification in ALEA_TirageAuSortChroniques

* Remove global vars : renaming

* Remove global vars : simplifying and clarifying a bit thermal noises

* Remove global vars : moving thermal noises to where it belongs + removing the useless AleaCoutDeProductionParPalier

* Remove global vars : forgot to update adequacy mode with the new way we add thermal noise

* Remove global vars : regression fix

* Remove global vars : extract thermal part of weekly problem building, because of adq patch

* [skip ci] Remove global vars : just clarifying

* Remove global vars : remove call to SIM_RenseignementProblemeHebdo in AdequacyPatchOptimization::solve

* Remove global vars : after previous commit removal, simplifications come naturally

* Remove global vars : some cleaning

* Fix/global var hydro modulable (#1614)

* Global vars - try remove HydrauliqueModulableQuotidien : simple renaming

* Global vars - try remove HydrauliqueModulableQuotidien : cleaning n class HydroManagement

* Global vars : rename and move down struct VALEURS_GENEREES_PAR_PAYS

* [skip ci] Global vars : small simplification

* remove TS number global var : we don't need timeseriesNumberYear in StudyRuntimeInfos (#1618)

* [skip ci] remove TS number global var : correction after review

* Update src/libs/antares/study/area/scratchpad.cpp

Co-authored-by: abdoulbari zakir <[email protected]>

* Apply a-zakir's remark

* Remove unused argument from StudyRuntimeInfos's ctor (#1634)

---------

Co-authored-by: Florian Omnès <[email protected]>
Co-authored-by: abdoulbari zakir <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants