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

Refactor common ClusterList class #1813

Merged
merged 58 commits into from
Dec 20, 2023
Merged
Changes from 1 commit
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
695d608
[DEV] Use vect instead of map
payetvin Dec 7, 2023
4ec8de1
[DEV] Change iterator typedef with vector iterator
payetvin Dec 8, 2023
dc72732
[DEV] FIx renewable
payetvin Dec 8, 2023
228280f
[DEV] More modif in common cluster_list
payetvin Dec 8, 2023
e68f7ed
[DEV] no error common clusterlist
payetvin Dec 8, 2023
8eeeb38
[FIX] compile cleaner-v20
payetvin Dec 8, 2023
0c2cc7b
[FIX] compile RenewableTSNumberData
payetvin Dec 8, 2023
5891cec
[FIX] compile thermal container
payetvin Dec 8, 2023
50d64f5
[FIX] compile thermal clusterlist
payetvin Dec 8, 2023
586c124
[FIX] compile ui, output var
payetvin Dec 8, 2023
d84cc59
[FIX] timeseries-numbers.cpp
payetvin Dec 8, 2023
35fda32
[FIX] common eco adq
payetvin Dec 8, 2023
1adcf49
[FIX] ui compile
payetvin Dec 8, 2023
039ddbf
[FIX] final compile
payetvin Dec 8, 2023
dd22673
[DEV] Sort cluster
payetvin Dec 8, 2023
d46d034
[DEV] removed byIndex
payetvin Dec 8, 2023
a7c8010
[DEV] small fix on add function
payetvin Dec 11, 2023
058a5b7
[DEV] code smells
payetvin Dec 11, 2023
66053ae
[DEV] comments 1
payetvin Dec 13, 2023
0249803
[DEV] change return type to bool
payetvin Dec 13, 2023
61fb467
[DEV] use std::all_of
payetvin Dec 13, 2023
758b349
[DEV] comments in renewables
payetvin Dec 13, 2023
ea38e89
[DEV] comments 2
payetvin Dec 13, 2023
2399d09
[DEV] refactor removeDisabledClusters by moving it in common
payetvin Dec 13, 2023
e653013
[DEV] comments ui
payetvin Dec 13, 2023
2c87746
[DEV] simplify common clusters : remove 2 useless find functions
guilpier-code Dec 14, 2023
04ce1bc
[DEV] simplify common clusters : "remove" function removal, because u…
guilpier-code Dec 14, 2023
0b42f10
[DEV] simplify common clusters : gathering find(...) functions defini…
guilpier-code Dec 18, 2023
c0de802
Merge branch 'fix/renew-lists' into fix/common-list
payetvin Dec 18, 2023
731076f
[DEV] simplify common clusters : we don't need to handle separately t…
guilpier-code Dec 18, 2023
7000fb1
Merge remote-tracking branch 'remotes/origin/fix/common-list' into fi…
guilpier-code Dec 18, 2023
64f31a3
[DEV] Using ranges and cpp 20 algo
payetvin Dec 18, 2023
e025643
Merge branch 'fix/common-list' of https://github.com/AntaresSimulator…
payetvin Dec 18, 2023
a200914
[DEV] renamed cluster into clusters
payetvin Dec 18, 2023
557201b
[FIX] compile
payetvin Dec 18, 2023
824282e
[DEV] Comments, removed groupCount
payetvin Dec 18, 2023
a52ea67
[DEV] more comments
payetvin Dec 18, 2023
92b2896
[DEV] rebuildIndex protected
payetvin Dec 18, 2023
e7f28c3
[DEV] doxygen comments
payetvin Dec 18, 2023
66a1852
[DEV] std::string typeID
payetvin Dec 18, 2023
1dffea3
[DEV] Remove non const each(predicate), use std::for_each for const one
payetvin Dec 18, 2023
2afb811
[DEV] add operator overload
payetvin Dec 18, 2023
334928e
[DEV] vector clusters protected
payetvin Dec 18, 2023
fff7e60
[DEV] study.cpp loop
payetvin Dec 18, 2023
f2244f7
[DEV] remove useless empty
payetvin Dec 19, 2023
716c858
[FIX] Adapt C++ 20 for win : make code compile when C++ 20 is forced
guilpier-code Dec 18, 2023
a653b1c
[DEV] simplify common clusters : use algorithm instead of a loop
guilpier-code Dec 19, 2023
e95d5a2
[DEV] using ranges allof
payetvin Dec 19, 2023
4177cc3
[DEV] remove reference for operator overload
payetvin Dec 19, 2023
e620d6e
[DEV] remove detach function
payetvin Dec 19, 2023
5545b12
Merge branch 'fix/common-list' of https://github.com/AntaresSimulator…
payetvin Dec 19, 2023
601da5a
[DEV] Add virtual func typeID back
payetvin Dec 20, 2023
ea797e6
[DEV] Clean save cluster function
payetvin Dec 20, 2023
9096788
[DEV] simplify ts function
payetvin Dec 20, 2023
f9a5d52
Revert "[DEV] remove detach function"
payetvin Dec 20, 2023
486a64a
[DEV] remove find(cluster)
payetvin Dec 20, 2023
dd96d28
[DEV] remove constructor destructor
payetvin Dec 20, 2023
8c55141
[DEV] remove detach function
payetvin Dec 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[DEV] removed byIndex
payetvin committed Dec 8, 2023
commit d46d03431001c5a0b943300d569677565b84394b
16 changes: 4 additions & 12 deletions src/libs/antares/study/parts/common/cluster_list.cpp
Original file line number Diff line number Diff line change
@@ -114,7 +114,6 @@ Data::ClusterList<ClusterT>::~ClusterList()
template<class ClusterT>
void ClusterList<ClusterT>::clear()
{
byIndex.clear();
cluster.clear();
}

@@ -190,20 +189,13 @@ void ClusterList<ClusterT>::storeTimeseriesNumbers(Solver::IResultWriter& writer
template<class ClusterT>
void ClusterList<ClusterT>::rebuildIndex()
{
byIndex.clear();
std::sort(cluster.begin(), cluster.end(), [&](const auto& a, const auto& b){
payetvin marked this conversation as resolved.
Show resolved Hide resolved
return a->id() < b->id();
});
if (not empty())
{
uint indx = 0;
byIndex.resize(size());
for (auto& c : cluster)
{
byIndex[indx] = c.get();
c->index = indx++;
}
}

uint indx = 0;
for (auto& c : cluster)
c->index = indx++;
}


2 changes: 0 additions & 2 deletions src/libs/antares/study/parts/common/cluster_list.h
Original file line number Diff line number Diff line change
@@ -215,8 +215,6 @@ class ClusterList
uint64_t memoryUsage() const;

public:
//! All clusters by their index
std::vector<ClusterT*> byIndex;
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, we remove the byIndex and turn cluster from a std::map into a std::vector.
It reminds me that we have the same situation in class AreaList.
Shouldn't we do the same thing for that class ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes open an issue please

//! All clusters
Vect cluster;
Copy link
Member

@sylvlecl sylvlecl Dec 13, 2023

Choose a reason for hiding this comment

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

I think we should push further the cleanup of this class:

  1. If the point of this PR is to clean up this class, we must make this private.
    The class is responsible to guarantee that the list remains sorted. It cannot guarantee this if we leave it public.
    We should also make rebuildIndex private, it's the responsibility of this class to maintain the internal consistency.

  2. (Optionally) As a whole, it may not be the best choice to remove the map and not remove the vector:
    a lot of operations of the class involve maintain the sort order and searching through it, which is what a map does naturally.

With a map, we can just expose a public getter (for example a range view) for when we want to iterate on clusters:

// assuming clusters is a map
<some range type here> getClusters() {
    return clusters | std::views::values;
}


Original file line number Diff line number Diff line change
@@ -79,12 +79,12 @@ void renewableTSNumberData::saveToINIFile(const Study& /* study */,
// Foreach renewable cluster...
for (uint y = 0; y != pTSNumberRules.height; ++y)
{
const uint val = get(pArea->renewable.list.byIndex[index], y);
const uint val = get(pArea->renewable.list.cluster[index].get(), y);
// Equals to zero means 'auto', which is the default mode
if (!val)
continue;
file << prefix << pArea->id << "," << y << ','
<< pArea->renewable.list.byIndex[index]->id() << " = " << val << '\n';
<< pArea->renewable.list.cluster[index]->id() << " = " << val << '\n';
}
JasonMarechal25 marked this conversation as resolved.
Show resolved Hide resolved
JasonMarechal25 marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
@@ -49,12 +49,12 @@ void thermalTSNumberData::saveToINIFile(const Study& /* study */,
// Foreach year ...
for (uint y = 0; y != pTSNumberRules.height; ++y)
{
const uint val = get(pArea->thermal.list.byIndex[index], y);
const uint val = get(pArea->thermal.list.cluster[index].get(), y);
// Equals to zero means 'auto', which is the default mode
if (!val)
continue;
file << prefix << pArea->id << "," << y << ','
<< pArea->thermal.list.byIndex[index]->id() << " = " << val << '\n';
<< pArea->thermal.list.cluster[index]->id() << " = " << val << '\n';
}
}
}
2 changes: 1 addition & 1 deletion src/libs/antares/study/study.cpp
Original file line number Diff line number Diff line change
@@ -1530,7 +1530,7 @@ void Study::computePThetaInfForThermalClusters() const
for (uint j = 0; j < area.thermal.list.size(); j++)
{
// Alias du cluster courant
auto& cluster = area.thermal.list.byIndex[j];
auto& cluster = area.thermal.list.cluster[j];
for (uint k = 0; k < HOURS_PER_YEAR; k++)
cluster->PthetaInf[k] = cluster->modulation[Data::thermalMinGenModulation][k]
* cluster->unitCount * cluster->nominalCapacity;
2 changes: 1 addition & 1 deletion src/solver/simulation/sim_calcul_economique.cpp
Original file line number Diff line number Diff line change
@@ -285,7 +285,7 @@ void SIM_InitialisationProblemeHebdo(Data::Study& study,

for (uint clusterIndex = 0; clusterIndex != area.thermal.list.size(); ++clusterIndex)
{
auto& cluster = *(area.thermal.list.byIndex[clusterIndex]);
auto& cluster = *(area.thermal.list.cluster[clusterIndex]);
pbPalier.NumeroDuPalierDansLEnsembleDesPaliersThermiques[clusterIndex]
= NombrePaliers + clusterIndex;
pbPalier.TailleUnitaireDUnGroupeDuPalierThermique[clusterIndex]
2 changes: 1 addition & 1 deletion src/solver/simulation/timeseries-numbers.cpp
Original file line number Diff line number Diff line change
@@ -747,7 +747,7 @@ Matrix<uint32_t>* getFirstTSnumberInterModalMatrixFoundInArea(
tsNumbersMtx = &(area.thermal.clusters[0]->series.timeseriesNumbers);
else if (isTSintermodal[ts_to_tsIndex.at(timeSeriesRenewable)]
&& area.renewable.list.size() > 0)
tsNumbersMtx = &(area.renewable.list.byIndex[0]->series.timeseriesNumbers);
tsNumbersMtx = &(area.renewable.list.cluster[0]->series.timeseriesNumbers);
}
assert(tsNumbersMtx);

2 changes: 1 addition & 1 deletion src/solver/variable/economy/productionByRenewablePlant.h
Original file line number Diff line number Diff line change
@@ -327,7 +327,7 @@ class ProductionByRenewablePlant : public Variable::IVariable<ProductionByRenewa
for (uint i = 0; i < pSize; ++i)
{
// Write the data for the current year
results.variableCaption = renewable.list.byIndex[i]->name();
results.variableCaption = renewable.list.cluster[i]->name();
results.variableUnit = VCardType::Unit();
pValuesForTheCurrentYear[numSpace][i].template buildAnnualSurveyReport<VCardType>(
results, fileLevel, precision);
2 changes: 1 addition & 1 deletion src/solver/variable/info.h
Original file line number Diff line number Diff line change
@@ -407,7 +407,7 @@ struct VariableAccessor<ResultsT, Category::dynamicColumns>
if (renewable_details)
{
auto& renewable = results.data.area->renewable;
results.variableCaption = renewable.list.byIndex[idx]->name();
results.variableCaption = renewable.list.cluster[idx]->name();
return true;
}
if (st_storage_details)
Original file line number Diff line number Diff line change
@@ -51,7 +51,7 @@ RenewableClusterSummarySingleArea::~RenewableClusterSummarySingleArea()
wxString RenewableClusterSummarySingleArea::rowCaption(int rowIndx) const
{
if (pArea)
return wxStringFromUTF8(pArea->renewable.list.byIndex[rowIndx]->name());
return wxStringFromUTF8(pArea->renewable.list.cluster[rowIndx]->name());
return wxEmptyString;
}

@@ -71,7 +71,7 @@ wxString RenewableClusterSummarySingleArea::columnCaption(int colIndx) const
wxString RenewableClusterSummarySingleArea::cellValue(int x, int y) const
{
Data::RenewableCluster* cluster = (pArea and (uint) y < pArea->renewable.list.size())
? pArea->renewable.list.byIndex[y]
? pArea->renewable.list.cluster[y].get()
: nullptr;
switch (x)
{
@@ -90,7 +90,7 @@ wxString RenewableClusterSummarySingleArea::cellValue(int x, int y) const
double RenewableClusterSummarySingleArea::cellNumericValue(int x, int y) const
{
Data::RenewableCluster* cluster = (pArea and (uint) y < pArea->renewable.list.size())
? pArea->renewable.list.byIndex[y]
? pArea->renewable.list.cluster[y].get()
: nullptr;
// gp : do we wish to have the line empty if cluster disabled
// if (!cluster->enabled)
@@ -112,7 +112,7 @@ double RenewableClusterSummarySingleArea::cellNumericValue(int x, int y) const
bool RenewableClusterSummarySingleArea::cellValue(int x, int y, const String& v)
{
auto* cluster = (pArea and (uint) y < pArea->renewable.list.size())
? pArea->renewable.list.byIndex[y]
? pArea->renewable.list.cluster[y].get()
: nullptr;

if (cluster)
Original file line number Diff line number Diff line change
@@ -51,7 +51,7 @@ ThermalClusterSummarySingleArea::~ThermalClusterSummarySingleArea()
wxString ThermalClusterSummarySingleArea::rowCaption(int rowIndx) const
{
if (pArea)
return wxStringFromUTF8(pArea->thermal.list.byIndex[rowIndx]->name());
return wxStringFromUTF8(pArea->thermal.list.cluster[rowIndx]->name());
return wxEmptyString;
}

@@ -82,7 +82,7 @@ wxString ThermalClusterSummarySingleArea::columnCaption(int colIndx) const
wxString ThermalClusterSummarySingleArea::cellValue(int x, int y) const
{
Data::ThermalCluster* cluster = (pArea and (uint) y < pArea->thermal.list.size())
? pArea->thermal.list.byIndex[y]
? pArea->thermal.list.cluster[y].get()
: nullptr;
if (!cluster->enabled)
return wxEmptyString;
@@ -125,7 +125,7 @@ wxString ThermalClusterSummarySingleArea::cellValue(int x, int y) const
double ThermalClusterSummarySingleArea::cellNumericValue(int x, int y) const
{
Data::ThermalCluster* cluster = (pArea and (uint) y < pArea->thermal.list.size())
? pArea->thermal.list.byIndex[y]
? pArea->thermal.list.cluster[y].get()
: nullptr;
if (!cluster->enabled)
return 0.;
@@ -168,7 +168,7 @@ double ThermalClusterSummarySingleArea::cellNumericValue(int x, int y) const
bool ThermalClusterSummarySingleArea::cellValue(int x, int y, const String& v)
{
auto* cluster = (pArea and (uint) y < pArea->thermal.list.size())
? pArea->thermal.list.byIndex[y]
? pArea->thermal.list.cluster[y].get()
: nullptr;

if (cluster)
Original file line number Diff line number Diff line change
@@ -54,7 +54,7 @@ wxString renewableScBuilderRenderer::rowCaption(int rowIndx) const
&& (uint)rowIndx < selectedArea()->renewable.list.size())
{
return wxString() << wxT(
" ") << wxStringFromUTF8(selectedArea()->renewable.list.byIndex[rowIndx]->name())
" ") << wxStringFromUTF8(selectedArea()->renewable.list.cluster[rowIndx]->name())
<< wxT(" ");
}
return wxEmptyString;
@@ -70,7 +70,7 @@ bool renewableScBuilderRenderer::cellValue(int x, int y, const String& value)
assert((uint)x < pRules->renewable[selectedArea()->index].height());
uint val = fromStringToTSnumber(value);
pRules->renewable[selectedArea()->index].setTSnumber(
selectedArea()->renewable.list.byIndex[y], x, val);
selectedArea()->renewable.list.cluster[y].get(), x, val);
return true;
}
return false;
Original file line number Diff line number Diff line change
@@ -54,7 +54,7 @@ wxString thermalScBuilderRenderer::rowCaption(int rowIndx) const
&& (uint)rowIndx < selectedArea()->thermal.list.size())
{
return wxString() << wxT(" ")
<< wxStringFromUTF8(selectedArea()->thermal.list.byIndex[rowIndx]->name())
<< wxStringFromUTF8(selectedArea()->thermal.list.cluster[rowIndx]->name())
<< wxT(" ");
}
return wxEmptyString;
@@ -69,7 +69,7 @@ bool thermalScBuilderRenderer::cellValue(int x, int y, const String& value)
assert((uint)y < pRules->thermal[selectedArea()->index].width());
assert((uint)x < pRules->thermal[selectedArea()->index].height());
uint val = fromStringToTSnumber(value);
pRules->thermal[selectedArea()->index].setTSnumber(selectedArea()->thermal.list.byIndex[y], x, val);
pRules->thermal[selectedArea()->index].setTSnumber(selectedArea()->thermal.list.cluster[y].get(), x, val);
return true;
}
return false;