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 max power : adding unit tests for timeseries numbers #1843

Merged
merged 141 commits into from
Mar 14, 2024

Conversation

guilpier-code
Copy link
Contributor

No description provided.

Copy link
Member

Choose a reason for hiding this comment

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

This file was moved to src/libs/antares/study/include/antares/study/parts/hydro/hydromaxtimeseriesreader.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This header was removed

Copy link
Member

Choose a reason for hiding this comment

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

Work ongoing for the documentation. Please revert this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

Copy link
Member

Choose a reason for hiding this comment

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

This file was moved to src/libs/antares/study/include/antares/study/scenario-builder/HydroMaxPowerTSNumberData.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

additional header removed

@@ -644,7 +644,7 @@ void SIM_RenseignementProblemeHebdo(const Study& study,
{
if (problem.CaracteristiquesHydrauliques[k].PresenceDHydrauliqueModulable > 0)
{
auto& area = *study.areas.byIndex[k];
auto& area = *study.areas.byIndex[k];
Copy link
Member

Choose a reason for hiding this comment

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

Don't add spaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Space removed

Comment on lines 319 to 326
// Hydro Max Power : Add hydro's max power number of TS in area ...
indexTS = ts_to_tsIndex.at(timeSeriesHydroMaxPower);
if (isTSintermodal[indexTS])
{
uint nbTimeSeries = area.hydro.series->maxPowerTScount();
listNumberTsOverArea.push_back(nbTimeSeries);
}

Copy link
Member

Choose a reason for hiding this comment

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

Duplicate of lines 311-317 above

Suggested change
// Hydro Max Power : Add hydro's max power number of TS in area ...
indexTS = ts_to_tsIndex.at(timeSeriesHydroMaxPower);
if (isTSintermodal[indexTS])
{
uint nbTimeSeries = area.hydro.series->maxPowerTScount();
listNumberTsOverArea.push_back(nbTimeSeries);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duplication removed

@pull-request-size pull-request-size bot added size/M and removed size/L labels Mar 14, 2024
@@ -178,7 +178,7 @@ this command allows to state, for each kind of time-series, whether it should be
the available set (be it ready-made or Antares-generated) _**OR**_ should take a user-defined value
(in the former case, the default "rand" value should be kept; in the latter, the value should be the reference number of the time-series to use). Multiple simulation profiles can be defined and archived. The default active profile gives the "rand" status for all time-series in all areas (full probabilistic simulation).

Regarding Hydro time-series, the scenario builder gives, in addition to the assignment of a specific number to use for the inflows time-series, the ability to define the initial reservoir level to use for each MC year.
Regarding Hydro time-series, the scenario builder gives, in addition to the assignment of a specific number to use for the inflows time-series, the ability to define the initial reservoir level to use for each MC year, also hydro max power scenario builder is available to support time-series for Maximum Generation and Maximum Pumping because the number of TS's for ROR, Hydro Storage and Minimum Generation can be different than the number of TS's for Maximum Generation and Maximum Pumping.
Copy link
Contributor Author

@guilpier-code guilpier-code Mar 11, 2024

Choose a reason for hiding this comment

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

Comment from a review before update with develop (when this change appeared) :
This text addition makes the sentence very long and this piece of documentation unclear.
We should prefer something like :

Regarding Hydro time-series, the scenario builder allows the user to choose, for a given year and area, a different time series whether we consider : 
- inflows, ROR and minimum generation
- initial level
- max power for generation and pumping
This implies that, inside one of the previous categories, the number of available time series is the same  

Copy link
Member

Choose a reason for hiding this comment

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

See #1990


Both types of data may come from any origin outside Antares, or may have been formerly generated by the Antares time-series stochastic generator and stored as input data on the user's request. Different ways to update data are:
ROR time-series and SP time-series may come from any origin outside Antares, or may have been formerly generated by the Antares time-series stochastic generator and stored as input data on the user's request. Minimum Generation, Maximum Generation and Maximum Pumping may come from any origin outside Antares, but they can not be generated by the Antares time-series stochastic generator. Different ways to update data are:
Copy link
Contributor Author

@guilpier-code guilpier-code Mar 12, 2024

Choose a reason for hiding this comment

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

Comment from a review before update with develop (when this change appeared) :
"The SP time-series" :
I guess SP stands for inflows or hydro storage (in Antares, it means the same thing [very unfortunate indeed, these 2 things should not be equivalent, because in real life, they aren't]).
I have never heard about SP.
So let's be more clear : rename SP into hydro storage


ROR time-series are defined at the hourly scale; each of the 8760 values represents the ROR power expected at a given hour, expressed in round number and in MW. The SP time-series are defined at the daily scale; each of the 365 values represents an overall SP energy expected in the day, expressed in round number and in MWh. These natural inflows are considered to be storable into a reservoir for later use.
ROR time-series are defined at the hourly scale; each of the 8760 values represents the ROR power expected at a given hour, expressed in round number and in MW. The SP time-series are defined at the daily scale; each of the 365 values represents an overall SP energy expected in the day, expressed in round number and in MWh. These natural inflows are considered to be storable into a reservoir for later use. The Minimum Generation time-series are defined at the hourly scale; each of the 8760 values represents the Minimum Generation power expected at a given hour expressed in round number and in MW. The Maximum Generation time-series are defined at the hourly scale; each of the 8760 values represents the Maximum Generation power expected at a given hour expressed in round number and in MW. The Maximum Pumping time-series are defined at the hourly scale; each of the 8760 values represents the Maximum Pumping power expected at a given hour expressed in round number and in MW.
Copy link
Contributor Author

@guilpier-code guilpier-code Mar 12, 2024

Choose a reason for hiding this comment

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

Comment from a review before update with develop (when this change appeared) :
starting from "The Minimum Generation time-series..." :
Following 3 phrases are the same, except for the quantity they concern.
One phrase is enough. Just add : "same for max generation and pumping power"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This header was removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

additional header removed

@@ -644,7 +644,7 @@ void SIM_RenseignementProblemeHebdo(const Study& study,
{
if (problem.CaracteristiquesHydrauliques[k].PresenceDHydrauliqueModulable > 0)
{
auto& area = *study.areas.byIndex[k];
auto& area = *study.areas.byIndex[k];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Space removed

Comment on lines 319 to 326
// Hydro Max Power : Add hydro's max power number of TS in area ...
indexTS = ts_to_tsIndex.at(timeSeriesHydroMaxPower);
if (isTSintermodal[indexTS])
{
uint nbTimeSeries = area.hydro.series->maxPowerTScount();
listNumberTsOverArea.push_back(nbTimeSeries);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duplication removed

Comment on lines +622 to +623
else if (isTSintermodal[ts_to_tsIndex.at(timeSeriesHydroMaxPower)])
tsNumbersMtx = &(area.hydro.series->timeseriesNumbersHydroMaxPower);
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

Comment on lines 153 to 158
// Hydro Max Power : set the nb of ready made TS
nbReadyMadeTS = 15;
area_1->hydro.series->resizeMaxPowerTS(nbReadyMadeTS);
area_2->hydro.series->resizeMaxPowerTS(nbReadyMadeTS);
area_3->hydro.series->resizeMaxPowerTS(nbReadyMadeTS);

Copy link
Member

Choose a reason for hiding this comment

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

Duplicate of lines above ⬆️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duplicated lines removed

area_3->hydro.series->resizeMaxPowerTS(hydroMaxPowerTSsize, 1);
area_3->resizeAllTimeseriesNumbers(1 + study->runtime->rangeLimits.year[rangeEnd]);

BOOST_CHECK(Generate(*study));
Copy link
Member

Choose a reason for hiding this comment

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

I kind of like Namespace::Generate, I find it beautiful

@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 14, 2024
@guilpier-code guilpier-code marked this pull request as ready for review March 14, 2024 14:59
Comment on lines 106 to 111
// Hydro Max Power: set the nb of ready made TS
nbReadyMadeTS = 15;
area_1->hydro.series->resizeMaxPowerTS(nbReadyMadeTS);
area_2->hydro.series->resizeMaxPowerTS(nbReadyMadeTS);
area_3->hydro.series->resizeMaxPowerTS(nbReadyMadeTS);

Copy link
Member

Choose a reason for hiding this comment

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

⬇️ Duplicate of lines 113-116

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duplicate lines removed

@guilpier-code guilpier-code requested a review from flomnes March 14, 2024 16:25
Copy link

sonarcloud bot commented Mar 14, 2024

@flomnes flomnes merged commit fa92e28 into develop Mar 14, 2024
5 checks passed
@flomnes flomnes deleted the feature/hydro-max-power-add-tests branch March 14, 2024 20:23
JasonMarechal25 pushed a commit that referenced this pull request Mar 20, 2024
Co-authored-by: NikolaIlic <[email protected]>
Co-authored-by: Milos-RTEi <[email protected]>
Co-authored-by: Milos <[email protected]>
Co-authored-by: nikolaredstork <[email protected]>
Co-authored-by: payetvin <[email protected]>
Co-authored-by: Florian Omnès <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants