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

Infeasibility more cleaning #2231

Merged
merged 18 commits into from
Jul 9, 2024
Merged

Conversation

guilpier-code
Copy link
Contributor

@guilpier-code guilpier-code commented Jul 3, 2024

We're on the road to make infeasibility analyzer more changeable, more precisely when adding a constraint type to the list of constraints to be detected in case of infeasibity.
This PR is a step towards this purpose.
It contains some heterogeneous simplifications and renaming.
In order to ease the review, some comments were added in this PR.

@guilpier-code guilpier-code force-pushed the fix/infeasibility-more-cleaning branch from 28afee5 to 8c17df6 Compare July 3, 2024 14:52
@guilpier-code guilpier-code force-pushed the fix/new-constraint-for-infeasibiliy branch from 18232c4 to 644335b Compare July 3, 2024 15:02
@guilpier-code guilpier-code force-pushed the fix/infeasibility-more-cleaning branch from 8c17df6 to bbdc230 Compare July 3, 2024 15:16
{
boost::algorithm::split_regex(nameComponents_, name_, boost::regex("::"));
return nameComponents_.size();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to return the size : this was returned only to test if this size is zero, but it can never be zero, as constraints are filtered based on regex pattern :

const std::string constraint_name_pattern = "^AreaHydroLevel::|::hourly::|::daily::|::weekly::|"
                                                "^FictiveLoads::|^Level::|"
                                                "^HydroPower::";

This forces the constraint's name to contain "::" and something else (like AreaHydroLevel, daily, ...).

{
const auto& begin = str.begin();
const auto& end = str.end();
std::vector<std::string> split_name;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we base the extraction of the string between angle bracket on the split algorithm, rather than the find algorithm. It makes code slightly more simple.
Error handling was a bit simplified.
In the end, function is shorter and easier to understand, I guess.

@@ -71,11 +72,8 @@ void InfeasibleProblemReport::sortConstraintsByType()
{
for (auto& c: constraints_)
{
if (c.extractComponentsFromName() == 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As already said above, extractComponentsFromName was returning the number of components a constraint's name is composed with, but this number can never be zero.
So the if statement was removed.

@@ -114,14 +116,12 @@ void InfeasibleProblemReport::logSuspiciousConstraints()
{
Antares::logs.error() << "* Binding constraints,";
}

Antares::logs.error() << "* Negative hurdle costs on lines with infinite capacity (rare).";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couldn't see any use for this log line :
it's printed in all cases of infeasibility, even if here are no link involved (indeed, hurdle costs are related to links).
Besides, it can be misleading for user, forced to wonder why he's got this message.

}

void InfeasibleProblemReport::prettyPrint()
{
sortConstraintsByType();
Copy link
Contributor Author

@guilpier-code guilpier-code Jul 4, 2024

Choose a reason for hiding this comment

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

The call to sortConstraintsByType() was moved above, with another call to a sort function.
Besides, this call is not related to client function's name, which is supposed to print only.

"BC::hourly::hour<36>",
"BC::daily::day<67>",
"BC::weekly::week<12>",
"BC-name-1::hourly::hour<36>",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I changed a bit the binding constraints' names.
Originally, these names started with "BC::", and it made think that BC names in mps files necessarily start with this prefix. This not the case, this prefix is the name of the BC.

@@ -86,6 +84,10 @@ void InfeasibleProblemReport::logSuspiciousConstraints()
{
Antares::logs.error() << c.prettyPrint();
}
}

void InfeasibleProblemReport::logInfeasibilityCauses()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we split logs into 2 separate functions :

  • one that logs suspicious constraints
  • one that logs of infeasibility causes

@guilpier-code guilpier-code marked this pull request as ready for review July 4, 2024 08:29
Base automatically changed from fix/new-constraint-for-infeasibiliy to develop July 8, 2024 08:30
@@ -75,67 +61,36 @@ class StringIsNotWellFormated: public std::runtime_error
}
};

std::string StringBetweenAngleBrackets(const std::string& str)
std::string StringBetweenAngleBrackets(const std::string& constraintName)
Copy link
Contributor

Choose a reason for hiding this comment

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

this function is agnostic to its input and therefore the param should not be renamed;

stream << std::string("Error the string: ") << std::quoted(str)
<< " does not contains the left angle bracket " << std::quoted("<");
throw StringIsNotWellFormated(stream.str());
err_msg += "constraint name '" + constraintName + "' misses '<' and/or '>' bracket";
Copy link
Contributor

Choose a reason for hiding this comment

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

or make it a class method

@@ -158,61 +113,46 @@ ConstraintType Constraint::getType() const
{
return ConstraintType::hydro_reservoir_level;
}
if (nameComponents_.at(0) == "HydroPower")
Copy link
Contributor

@a-zakir a-zakir Jul 8, 2024

Choose a reason for hiding this comment

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

it's works because Antares provides constraints and vars names, trimmed!

@flomnes flomnes merged commit 3fc885a into develop Jul 9, 2024
5 of 7 checks passed
@flomnes flomnes deleted the fix/infeasibility-more-cleaning branch July 9, 2024 13:40
Copy link

sonarqubecloud bot commented Jul 9, 2024

meslubi2021 added a commit to Unity-Energy-and-renewable-energy/Antares_Simulator that referenced this pull request Jul 17, 2024
* Collect hydro validation errors (AntaresSimulatorTeam#2204)

collect *n* hydro validation errors (10 per area ) before exiting

---------

Co-authored-by: Florian Omnès <[email protected]>

* Remove actions dependencies using node js 16 (AntaresSimulatorTeam#2215)

> Node.js 16 actions are deprecated. Please update the following actions
to use Node.js 20: notiz-dev/github-action-json-property@release. For
more information see:
https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Move TS number print (AntaresSimulatorTeam#2228)

Purpose : moving the code that prints the TS numbers on disk : due to
recent changes it was moved after the loop through MC years. So, we have
to wait until simulation ends to know which TS numbers where drawn. So
we move that code back.

* Infeability analyzer : renaming (AntaresSimulatorTeam#2225)

Purpose : as the title says

This PR is attached to ticket [ticket
ANT-1825](https://gopro-tickets.rte-france.com/browse/ANT-1825).
Some improvements were made or tried by taking care of the ticket.
The result is this PR.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Always run clang-format on PR (AntaresSimulatorTeam#2230)

* Add changelog for minor versions (AntaresSimulatorTeam#2229)

- v8.8.6
- v8.6.8

---------

Co-authored-by: Abdoulbari Zaher <[email protected]>
Co-authored-by: Jason Maréchal <[email protected]>

* Infeasibility analyzer : small simplifications (AntaresSimulatorTeam#2226)

Purpose : as the title says

This PR is attached to ticket [ticket
ANT-1825](https://gopro-tickets.rte-france.com/browse/ANT-1825).
Some improvements were made or tried by taking care of the ticket.
The result is this PR.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Fix compile, add boost header in CMakelists (AntaresSimulatorTeam#2233)

* STS: Withdrawal efficiency [ANT-1862] (AntaresSimulatorTeam#2223)

Add a efficiencywithdrawal property to short term storage
This mimics the already existing efficiency for injection

* Infeasibility analyzer : HydroPower constraint (AntaresSimulatorTeam#2227) [ANT-1825]

Add constraint that can possibily lead to infeasibilities. HydroPower is the constraint on the sum of generated hydro energy over the week for a given area. The provided RHS in some cases makes the problem infeasible.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Compilation warnings (AntaresSimulatorTeam#2237)

close  AntaresSimulatorTeam#2236

* Version 9.2 (AntaresSimulatorTeam#2240)

* Fix sonarcloud job (AntaresSimulatorTeam#2246)

* Adequacy Patch regression [ANT-1845] (AntaresSimulatorTeam#2235)

* 9.2 rc 1 (AntaresSimulatorTeam#2247)

* Keep using node js 16 on centos CI (AntaresSimulatorTeam#2248)

* Infeasibility more cleaning (AntaresSimulatorTeam#2231)

We're on the road to make **infeasibility analyzer** more changeable,
more precisely when adding a constraint type to the list of constraints
to be detected in case of infeasibity.
This PR is a step towards this purpose.  
It contains some heterogeneous simplifications and renaming.
In order to ease the review, some comments were added in this PR.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Document clang-format (AntaresSimulatorTeam#2243)

Add version number to avoid confusion.

* Link TS generation : splitting into multiple files (AntaresSimulatorTeam#2171)

This PR aims at moving some code about link TS generation, from **main**
program to new source files.
It follows PR AntaresSimulatorTeam#2155 

To be done : 

- [x] move headers (*.h) to the right place
- [x] update this branch with its base branch (@flomnes made a change on
base branch)

We may also take advantage of this PR to : 
- [ ] Thermal TS generation :
- [ ] avoid loading the **whole study** when **no** thermal TS
generation is required (this is currently the case)
- [ ] move implementation details about thermal TS generation from
**main** program to new source files
- [ ] Separate loading / extracting data from study and generate the TS

---------

Co-authored-by: Florian OMNES <[email protected]>
Co-authored-by: Florian OMNES <[email protected]>

* Explain why MPS are named when the problem is infeasible (AntaresSimulatorTeam#2250)

* Add tests in CI for version 9.2 (AntaresSimulatorTeam#2241)

Co-authored-by: Florian OMNES <[email protected]>

* Rename test-platform to os for windows CI (AntaresSimulatorTeam#2253)

* Fix bug hydro heuristic with mingen (ANT-1825) (AntaresSimulatorTeam#2258)

Removing lines that create infeasibility and that are not necessary.

---------

Co-authored-by: Juliette-Gerbaux <[email protected]>
Co-authored-by: Florian OMNES <[email protected]>

* Remove useless fwd declaration

---------

Co-authored-by: Abdoulbari Zaher <[email protected]>
Co-authored-by: Florian Omnès <[email protected]>
Co-authored-by: payetvin <[email protected]>
Co-authored-by: guilpier-code <[email protected]>
Co-authored-by: Florian OMNES <[email protected]>
Co-authored-by: Jason Maréchal <[email protected]>
Co-authored-by: Juliette-Gerbaux <[email protected]>
Co-authored-by: Juliette-Gerbaux <[email protected]>
meslubi2021 added a commit to Unity-Energy-and-renewable-energy/Antares_Simulator that referenced this pull request Jul 17, 2024
* Collect hydro validation errors (AntaresSimulatorTeam#2204)

collect *n* hydro validation errors (10 per area ) before exiting

---------

Co-authored-by: Florian Omnès <[email protected]>

* Remove actions dependencies using node js 16 (AntaresSimulatorTeam#2215)

> Node.js 16 actions are deprecated. Please update the following actions
to use Node.js 20: notiz-dev/github-action-json-property@release. For
more information see:
https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Move TS number print (AntaresSimulatorTeam#2228)

Purpose : moving the code that prints the TS numbers on disk : due to
recent changes it was moved after the loop through MC years. So, we have
to wait until simulation ends to know which TS numbers where drawn. So
we move that code back.

* Infeability analyzer : renaming (AntaresSimulatorTeam#2225)

Purpose : as the title says

This PR is attached to ticket [ticket
ANT-1825](https://gopro-tickets.rte-france.com/browse/ANT-1825).
Some improvements were made or tried by taking care of the ticket.
The result is this PR.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Always run clang-format on PR (AntaresSimulatorTeam#2230)

* Add changelog for minor versions (AntaresSimulatorTeam#2229)

- v8.8.6
- v8.6.8

---------

Co-authored-by: Abdoulbari Zaher <[email protected]>
Co-authored-by: Jason Maréchal <[email protected]>

* Infeasibility analyzer : small simplifications (AntaresSimulatorTeam#2226)

Purpose : as the title says

This PR is attached to ticket [ticket
ANT-1825](https://gopro-tickets.rte-france.com/browse/ANT-1825).
Some improvements were made or tried by taking care of the ticket.
The result is this PR.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Fix compile, add boost header in CMakelists (AntaresSimulatorTeam#2233)

* STS: Withdrawal efficiency [ANT-1862] (AntaresSimulatorTeam#2223)

Add a efficiencywithdrawal property to short term storage
This mimics the already existing efficiency for injection

* Infeasibility analyzer : HydroPower constraint (AntaresSimulatorTeam#2227) [ANT-1825]

Add constraint that can possibily lead to infeasibilities. HydroPower is the constraint on the sum of generated hydro energy over the week for a given area. The provided RHS in some cases makes the problem infeasible.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Compilation warnings (AntaresSimulatorTeam#2237)

close  AntaresSimulatorTeam#2236

* Version 9.2 (AntaresSimulatorTeam#2240)

* Fix sonarcloud job (AntaresSimulatorTeam#2246)

* Adequacy Patch regression [ANT-1845] (AntaresSimulatorTeam#2235)

* 9.2 rc 1 (AntaresSimulatorTeam#2247)

* Keep using node js 16 on centos CI (AntaresSimulatorTeam#2248)

* Infeasibility more cleaning (AntaresSimulatorTeam#2231)

We're on the road to make **infeasibility analyzer** more changeable,
more precisely when adding a constraint type to the list of constraints
to be detected in case of infeasibity.
This PR is a step towards this purpose.  
It contains some heterogeneous simplifications and renaming.
In order to ease the review, some comments were added in this PR.

---------

Co-authored-by: Florian OMNES <[email protected]>

* Document clang-format (AntaresSimulatorTeam#2243)

Add version number to avoid confusion.

* Link TS generation : splitting into multiple files (AntaresSimulatorTeam#2171)

This PR aims at moving some code about link TS generation, from **main**
program to new source files.
It follows PR AntaresSimulatorTeam#2155 

To be done : 

- [x] move headers (*.h) to the right place
- [x] update this branch with its base branch (@flomnes made a change on
base branch)

We may also take advantage of this PR to : 
- [ ] Thermal TS generation :
- [ ] avoid loading the **whole study** when **no** thermal TS
generation is required (this is currently the case)
- [ ] move implementation details about thermal TS generation from
**main** program to new source files
- [ ] Separate loading / extracting data from study and generate the TS

---------

Co-authored-by: Florian OMNES <[email protected]>
Co-authored-by: Florian OMNES <[email protected]>

* Explain why MPS are named when the problem is infeasible (AntaresSimulatorTeam#2250)

* Add tests in CI for version 9.2 (AntaresSimulatorTeam#2241)

Co-authored-by: Florian OMNES <[email protected]>

* Rename test-platform to os for windows CI (AntaresSimulatorTeam#2253)

* Fix bug hydro heuristic with mingen (ANT-1825) (AntaresSimulatorTeam#2258)

Removing lines that create infeasibility and that are not necessary.

---------

Co-authored-by: Juliette-Gerbaux <[email protected]>
Co-authored-by: Florian OMNES <[email protected]>

* Infeasibility anaylsis : make it more changeable (AntaresSimulatorTeam#2232)

Create classes for each constraint type, with a common interface. Additional tests to come.

---------

Co-authored-by: Florian OMNES <[email protected]>
Co-authored-by: Florian Omnès <[email protected]>

* Remove manual dynamic memory usage (AntaresSimulatorTeam#2254)

Replaced new[] with vectors
Made RunTimeInfos static

---------

Co-authored-by: Abdoulbari Zaher <[email protected]>
Co-authored-by: Florian Omnès <[email protected]>
Co-authored-by: payetvin <[email protected]>
Co-authored-by: guilpier-code <[email protected]>
Co-authored-by: Florian OMNES <[email protected]>
Co-authored-by: Jason Maréchal <[email protected]>
Co-authored-by: Juliette-Gerbaux <[email protected]>
Co-authored-by: Juliette-Gerbaux <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants