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

Add ts-generation for links [ANT-1084] #1986

Merged
merged 114 commits into from
Jun 21, 2024
Merged

Conversation

payetvin
Copy link
Contributor

No description provided.

@payetvin payetvin added the enhancement New feature or request label Mar 12, 2024
@payetvin payetvin self-assigned this Mar 12, 2024
Copy link
Member

@flomnes flomnes left a comment

Choose a reason for hiding this comment

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

Pros

  • We know that foDuration and its friends must of size 8760.

Cons

  • On my machine, sizeof(thermalInterface) = 420512, which is huge. With this, I could reproduce a segfault by just declaring thermalStructure t[24]; on the stack.

Maybe you could wrap a std::vector inside a class that does not allow resizing ?

class YearlyTS {
public:
YearlyTS() : mVec(8760) {}
double& operator[](std::size_t idx) { return mVec[idx];}
// etc.
// Do *NOT* expose resize, clear, push_back, etc.
private:
  std::vector<double> mVec;
};

struct newStructure
{
    YearlyTS foDuration;
    YearlyTS poDuration;
    YearlyTS foRate;
    YearlyTS poRate;
    YearlyTS npoMin;
    YearlyTS npoMax;

    unsigned unitCount;
    float nominalCapacity;

    double forcedVolatility;
    double plannedVolatility;

    Data::ThermalLaw forcedLaw = Data::thermalLawUniform;
    Data::ThermalLaw plannedLaw = Data::thermalLawUniform;
};

So you get both the "small object on the stack" and "fixed size" aspects ? Note that on my machine, sizeof(newStructure) = 176.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 12, 2024
@pull-request-size pull-request-size bot added size/M and removed size/L labels Mar 13, 2024
@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 13, 2024
@pull-request-size pull-request-size bot added size/M and removed size/L labels Mar 13, 2024
@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 14, 2024
@@ -1,5 +1,26 @@
# Migration guides
This is a list of all recent changes that came with new Antares Simulator features. The main goal of this document is to lower the costs of changing existing interfaces, both GUI and scripts.

## v9.2.0
### (TS-generator only) TS generation for links
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the quantity being generated for links ?
capacity ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Copy link
Contributor

Choose a reason for hiding this comment

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

Then we should say it.
Otherwise it's unclear


## v9.2.0
### (TS-generator only) TS generation for links
In files input/links/<link1>/properties.ini, add the following properties
Copy link
Contributor

Choose a reason for hiding this comment

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

This doc is not very clear to me, especially when opening it with a markdown reader (I use VS Code).
This is a long list of different things.
Apparently, there are 4 files concerned by the links capacities TS generation.
So it should be separated into 4 parts.

src/tools/ts-generator/main.cpp Show resolved Hide resolved
src/tools/ts-generator/main.cpp Show resolved Hide resolved
src/tools/ts-generator/main.cpp Show resolved Hide resolved
src/tools/ts-generator/main.cpp Show resolved Hide resolved
src/solver/ts-generator/availability.cpp Show resolved Hide resolved

for (const auto& link: links)
{
Data::TimeSeries ts(link->timeseriesNumbers);
Copy link
Contributor

Choose a reason for hiding this comment

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

About Data::TimeSeries ts(link->timeseriesNumbers); :
We don't need the time series numbers in the context of TS generation.
Actually we don't even need a Data::TimeSeries here.
A simple Matrix is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do

src/solver/ts-generator/availability.cpp Show resolved Hide resolved
@@ -1,5 +1,26 @@
# Migration guides
This is a list of all recent changes that came with new Antares Simulator features. The main goal of this document is to lower the costs of changing existing interfaces, both GUI and scripts.

## v9.2.0
### (TS-generator only) TS generation for links
Copy link
Contributor

Choose a reason for hiding this comment

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

Then we should say it.
Otherwise it's unclear

src/tools/ts-generator/main.cpp Show resolved Hide resolved
@@ -539,6 +539,10 @@ static bool SGDIntLoadFamily_General(Parameters& d,
{
return value.to<uint>(d.nbTimeSeriesSolar);
}
if (key == "nbtimeserieslinks")
Copy link
Contributor

Choose a reason for hiding this comment

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

We know generating TS for links should be fully separated from the Antares solver.
What is true for the code is true for data as well.
For now data required generating TS for links are mixed data with the study data.
Here is a good example : nbtimeserieslinks is still held by generaldata.ini. Same for seed_links.
Eventually, we should separate data for TS generation form study data.

Copy link
Member

Choose a reason for hiding this comment

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

See your PR @guilpier-code

src/libs/antares/study/area/links.cpp Show resolved Hide resolved
@flomnes flomnes changed the title Add ts-generation for links Add ts-generation for links [ANT-1084] Jun 10, 2024
@flomnes flomnes dismissed guilpier-code’s stale review June 21, 2024 10:00

@guilpier made a dedicated PR to fix all issues

@flomnes flomnes merged commit 9e6cdcc into develop Jun 21, 2024
6 checks passed
@flomnes flomnes deleted the feature/thermal-interface branch June 21, 2024 10:01
Copy link

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

flomnes added a commit that referenced this pull request Jun 28, 2024
The aim of this PR is removing from **Antares solver** the code related
to TS generation associated to **links** capacity.
It follows PR #1986 

Note that : 
- Loading the study is now no longer required to generate TS associated
to links.
- Handling errors related to each link's TS generation was simplified : 
For a link, if we meet a problem while loading data required to
generated its TS, a **warning** is raised and the TS generation for this
link will be skipped.

What was done : 
- Reading data for links TS generation :  
   We now don't need the study to be loaded in order to generate TS. 
   Some new code retrieves data from study
- Generating TS associated to links : 
  Code of base branch was adapted to use data having a different format.
- Code related to link TS generation in **solver** was removed.
An exception though : due to new data (related to links TS generation)
next to data consumed by solver, we had to keep special skipping code
for it, otherwise we get reading errors.

---------

Co-authored-by: Florian OMNES <[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/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants