Feature/network cloning #28

ianmnz · 2024-03-22T16:32:26Z

An initial merge for the investment pathway. By this stage, it works even if in a very artisanal way (not user friendly)

tbittar · 2024-07-12T07:25:24Z

src/andromede/model/constraint.py

-        if isinstance(expression, ComparisonNode):
-            if lower_bound is not None or upper_bound is not None:
+        if isinstance(self.expression, ComparisonNode):
+            if not is_unbound(self.lower_bound) or not is_unbound(self.upper_bound):


I think with the definition of is_unbound, I am allowed to use +inf in both lower bound and upper bound without raising an exception (idem with -inf). I think the check should be tighter ie. self.lower_bound != -inf and self.upper_bound != +inf

For the ComparisonNode part, it won't be a problem because some lines below we set the bounds ourselves.
However, for LiteralNodes and ParameterNodes (else statement), I agree with you. I will fix it

tbittar · 2024-07-12T07:34:22Z

src/andromede/model/constraint.py

-                self.upper_bound = upper_bound
-            else:
-                self.upper_bound = literal(float("inf"))
+    def replicate(self, /, **changes: Any) -> "Constraint":


Is the backslash useful ?

In terms of code, no. It's for the developers.
It's a pythonic way of saying that there are no positional arguments allowed, only keyword arguments

tbittar · 2024-07-12T07:50:44Z

src/andromede/simulation/benders_decomposed.py

-                    solver_var_info.name
-                ] = solver_var_info.column_id
-                candidates.add(solver_var_info.name)
+            problem_to_candidates["master"][


Can you reexplain why it is not necessary anymore to filter variables that are in objective ?

To be honest, I don't remember anymore 😅
I had some problems with sending the right variable to the master, but I don't remember exactly

tbittar · 2024-07-12T07:53:43Z

src/andromede/simulation/benders_decomposed.py

@@ -126,7 +123,7 @@ def export_options(
            "TRACE": True,
            "SLAVE_WEIGHT": "CONSTANT",
            "SLAVE_WEIGHT_VALUE": 1,
-            "MASTER_NAME": "master",
+            "MASTER_NAME": f"{self.master.name}",
            "STRUCTURE_FILE": "structure.txt",


Why not use a variable to store structure file name, similarly as for master, as it is reused in line 158 ?

We could. It would just have to pass down to the BendersDecomposedProblem as a parameter (with "structure.txt" being the default value). I'll do it

tbittar · 2024-07-12T08:10:26Z

src/andromede/simulation/benders_decomposed.py

+    subproblems = []  # Benders Decomposed Sub-problems
+
+    for tree_node in decision_tree_root.traverse():
+        suffix = f"_{tree_node.id}" if decision_tree_root.size > 1 else ""


What does the case decision_tree_root.size == 1 mean ? Why don't we add the corresponding suffix ?

In this case, it would represent the current behavior, so no pathway.
It would be unnecessary to add a suffix to master and sub-problems since they all belong to the same node

tbittar · 2024-07-12T11:33:09Z

tests/functional/test_investment_pathway.py

+    return node
+
+
+def test_investment_pathway_on_sequential_nodes(


Maybe try to refactorize this test a bit so that we see more clearly the general structure : generate database, create coupling model / network, replicate on nodes (almost duplicated code on parent and child), decision tree building, ...

Also write a comment that describes the test and so that we understand the expected solution at glance

tbittar · 2024-07-12T11:36:11Z

tests/functional/test_investment_pathway.py

+    This example models a case where investment decisions have to be made in 2030 and 2040.
+        - In 2030, we have full knowledge of the existing assets
+        - In 2040, two possible hypothesis are possible :
+            - P=0.2 => A case where there is no change in the generation assets since 2030 (except te potential investment in 2030)


the potential

tbittar · 2024-07-12T11:40:33Z

src/andromede/simulation/benders_decomposed.py

        solver_id=solver_id,
-        problem_strategy=InvestmentProblemStrategy(),
+        build_strategy=InvestmentProblemStrategy(),
+        risk_strategy=ExpectedValue(0.0),


This means that the objective is zero, right ? By doing so, if the user specify an objective contribution in the coupling model (which should not occur in practice...), the build_problem will create an objective with coefficient values equal to zero ? Could this cause issues for the solvers ?

Should we rather put a check in the coupling model to ensure that there is no objective ? But this means we are able to detect that some model is a coupling model within a tree, not easy...

Yeah, that's right. The objective should be zero for the coupling model (at least for now?)
I don't know if coefficients 0 generate problem to the solvers, but we could check if a expression is 0 in the AST and skip it in the build problem ?

tbittar · 2024-07-12T11:42:15Z

tests/functional/test_investment_pathway.py

+
+    But here as we solve on the tree, we need to find the best solution in expectation on the set of paths in the tree.
+
+    Case 1 :    prob    |    investment   |  operational


Nice, very clear !

tbittar · 2024-07-12T11:45:03Z

tests/functional/test_investment_pathway.py

+                context=ProblemContext.INVESTMENT,
+            ),
+        ],
+        constraints=[


This constraints linking invested_capa and delta_invest seems to be always the same, maybe a next step in the work (not in this PR obviously) could be to automatically generate those from the tree structure ?

Indeed! Automate it would be the next step to allow this to become more user friendly anyway and less artisanal

tbittar · 2024-07-12T11:50:14Z

What do you mean by it does not take into account the probability of the child nodes ? As it seems you use the risk_strategy argument to modulate the objective function with the probability of the node ?

ianmnz · 2024-07-16T08:45:47Z

What do you mean by it does not take into account the probability of the child nodes ? As it seems you use the risk_strategy argument to modulate the objective function with the probability of the node ?

It was a legacy description, sorry

* Define tree structure for time blocks and data Define tree structure * Improve test description * Create class to store configuration of a resolution node * Work in progress * Design the construction of network on decision tree * Work in progress * Add comments * Fix rebase conflict * Feature/network cloning (#28) * Move pathway tests to functional folder * Add replicate method to classes to be able to copy them to tree * Remove integration test from functional xpansion test * Modify DecisionTree * Move decision_tree test to unit tests folder * Ideas for the coupling_model * Write subproblem with tree * Add first pathway test * Add three node test for pathway * Add prob to tree nodes for expectation computation * Remove test debug init * Add prob to decision tree node * Add ExpectedValueStrategy to Benders * Retour of PR comments * Squashed merged commit from 'main' commit 7fa89213801af1f57e340bcc40b0dac1eb6c4af7 Merge: 964297a 0074e94 Author: Ian Menezes <ian.menezes@rte-france.com> Date: Wed Jul 24 11:43:38 2024 +0200 Merge branch 'main' into feature/investment_pathway commit 0074e94 Author: ianmnz <ian.menezes@rte-france.com> Date: Wed Jul 24 10:18:58 2024 +0200 Add scenario benders test, bounds validation, and support for bool variables (#46) * Cherry-picked solver var name prefixes and suffixes * Added scenario tests for benders decomposed * Added BoolVar type * Added check for upper-lower variable bounds * Added index struct check for variable naming * Added check for inverted variable bounds commit a7fab1b Author: Yann-Temudjin <160221141+Yann-Temudjin@users.noreply.github.com> Date: Mon Jun 17 17:02:47 2024 +0200 Short term storage complex (#27) * test Quota CO2 * reformatage du fichier test_quota_co2 * Ajout de Short term storage complex dans les modeles standard * Ajout du test electrolyzer n inputs * fix taking in account the comments on the pull request * Ajout du isclose et modification des tests pour les electrolyzer n_inputs * Fixed test 3 and 4 of the electrolyzer_n_inputs * Model SHORT_TERM_STORAGE_COMPLEX avec le même test que le SIMPLE * fixed test short term storage * fixed generate data for short term storage complex * fixed import order * Formatting change --------- Co-authored-by: Thomas Bittar <thomas.bittar@rte-france.com> commit 12ccfd7 Author: Louis Chouane <116257827+ChouaneLouis@users.noreply.github.com> Date: Mon Jun 3 10:56:05 2024 +0200 Re-implement functional tests in yaml + multiple fixes (#35) Solved multiple issues: - parsing of shift expressions was wrong - integer variables were not correctly transferred to solver - improved some error messages at parsing time Co-authored-by: vargastat <tatiana.vargas@rte-france.com> commit 346d852 Merge: 1c1bbd2 52b5b62 Author: ianmnz <ian.menezes@rte-france.com> Date: Fri May 17 14:08:37 2024 +0200 Merge pull request #34 from AntaresSimulatorTeam/test_performance_large_ast Performance test for deep AST commit 52b5b62 Merge: dc4940d 1c1bbd2 Author: Ian Menezes <ian.menezes@rte-france.com> Date: Fri May 17 13:46:24 2024 +0200 Merge branch 'main' into test_performance_large_ast commit dc4940d Author: Ian Menezes <ian.menezes@rte-france.com> Date: Fri May 17 13:21:28 2024 +0200 Add RecursionError raise check for tests commit f85d56e Author: Ian Menezes <ian.menezes@rte-france.com> Date: Thu May 16 16:43:10 2024 +0200 Remove unused generate_random_data function commit ead9b99 Author: Ian Menezes <ian.menezes@rte-france.com> Date: Thu May 16 16:19:38 2024 +0200 Performance test: check large nb of port connections commit 1c1bbd2 Author: vargastat <130074062+vargastat@users.noreply.github.com> Date: Mon May 13 13:59:08 2024 +0200 Yaml format for components and TSV format for timeseries (#37) commit acc77e8 Author: Ian Menezes <ian.menezes@rte-france.com> Date: Fri Apr 26 11:43:55 2024 +0200 Performance test: check deep AST using sum() operator commit 1221880 Merge: 9f419cd ecd3e1d Author: Florian Omnès <florian.omnes@rte-france.com> Date: Tue Apr 23 17:36:48 2024 +0200 Merge pull request #32 from AntaresSimulatorTeam/feature/upgrade-ortools Upgrade ortools in requirements commit ecd3e1d Author: Thomas Bittar <thomas.bittar@rte-france.com> Date: Tue Apr 23 17:28:38 2024 +0200 Remove unnecessary requirements commit be47c07 Author: Thomas Bittar <thomas.bittar@rte-france.com> Date: Tue Apr 23 17:14:10 2024 +0200 Upgrade ortools in requirements commit 9f419cd Author: Juliette-Gerbaux <130555142+Juliette-Gerbaux@users.noreply.github.com> Date: Fri Apr 19 09:46:19 2024 +0200 Models/add new model (#30) * Add variant for electrolyzer subject to renewable * Add model hydro valley --------- Co-authored-by: Juliette-Gerbaux <juliette.gerbaux_externe@rte-france.com> commit fb547dd Author: Yann-Temudjin <160221141+Yann-Temudjin@users.noreply.github.com> Date: Fri Apr 19 09:43:20 2024 +0200 Model test yaml (#29) * test electrolyzer_n_inputs with yaml models * quota C02 test with yaml model * print to test the path * fixed path for the first n_input_electrolyzer_yaml test * fix of libs path and move of the tests to models * factorisation, removal of unused import and move of test files * reformat test * move tests of model to tests/model * Fix formatting * re-added libs_dir to model/conftest.py * added type declaration and removed unused argument --------- Co-authored-by: Thomas Bittar <thomas.bittar@rte-france.com> commit 7c7d2d5 Author: Sylvain Leclerc <sylvain.leclerc@rte-france.com> Date: Fri Mar 29 17:17:58 2024 +0100 Test case for AC link and PST models (#2) Addition of an ac.yml library, and associated test cases. Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com> commit f9f9b3a Author: Yann-Temudjin <160221141+Yann-Temudjin@users.noreply.github.com> Date: Fri Mar 22 15:34:36 2024 +0100 test Quota CO2 (#17) * test Quota CO2 * reformatage du fichier test_quota_co2 * Ajout de Short term storage complex dans les modeles standard * Ajout du test electrolyzer n inputs * fix taking in account the comments on the pull request * Ajout du isclose et modification des tests pour les electrolyzer n_inputs * Fixed test 3 and 4 of the electrolyzer_n_inputs commit 94f13b1 Author: Sylvain Leclerc <sylvain.leclerc@rte-france.com> Date: Wed Mar 20 17:24:33 2024 +0100 Model library format parsing (#11) First implementation of library format parsing, including expressions parsing. Implementation notes: - ANTLR4 is used for expression parsing - pydantic together with PyYaml are used for yaml parsing - An additional layer of "unresolved" objects, that are the 1-to-1 image of the input data, is introduced. Once everything is read, we can resolve all objects to actually use them for computation. Note: - Last additions for investment studies have not yet been included ("context" and separation between 2 objectives). --------- Signed-off-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com> Signed-off-by: vargastat <tatiana.vargas@rte-france.com> Co-authored-by: vargastat <tatiana.vargas@rte-france.com> commit c4b0abd Author: Juliette-Gerbaux <130555142+Juliette-Gerbaux@users.noreply.github.com> Date: Tue Mar 19 11:34:37 2024 +0100 Add flow-based models (#23) * Add flow based models * Add new model for flow based with net positions * Delete useless model * Correct errors * Rename nodes to real countries to clarify differences between models * Small corrections --------- Co-authored-by: Juliette-Gerbaux <juliette.gerbaux_externe@rte-france.com> commit dfc8871 Author: Juliette-Gerbaux <130555142+Juliette-Gerbaux@users.noreply.github.com> Date: Tue Mar 19 11:34:16 2024 +0100 Add models representing binding constrains (#25) * Correct error in stock model * Add model constraining total imports * Add nuclear modulation model * Add model with minimum electric vehicle load * Add hvdc model * Delete models because more efficient models are available with ports * Small corrections on names and temporal sums * Change model to take into account total import on a node * Small corrections --------- Co-authored-by: Juliette-Gerbaux <juliette.gerbaux_externe@rte-france.com> commit 2e2f8bb Author: tbittar <thomas.bittar@gmail.com> Date: Fri Mar 15 16:40:17 2024 +0100 Correct electrolyzer model typos (#26) * Fix sign mistakes * Fix sign and arrow direction --------- Co-authored-by: Thomas Bittar <thomas.bittar@rte-france.com> * Fixed var name tree prefix * Cherry-picked benders_decomposed debug mode * Feature/automatic coupling model (#52) * Refactored Port classes to port.py * Add automatic decision tree node coupling * Cherry-picked benders_decomposed debug mode * Fixed var name tree prefix * Move invest pathway model to test only * Allow for more flexible expressions between tree nodes * Reset to a more rigid type of coupling constraints * Fixed unit test errors * Fixed expected value risk management missing * Fix antlr differences * Fixed from PR comments --------- Co-authored-by: ianmnz <ian.menezes@rte-france.com>

ianmnz added 6 commits March 21, 2024 14:08

Move pathway tests to functional folder

5d760f5

Add replicate method to classes to be able to copy them to tree

fba71cd

Remove integration test from functional xpansion test

3e92e20

Modify DecisionTree

d12d77f

Move decision_tree test to unit tests folder

c42c48f

Ideas for the coupling_model

35b6758

ianmnz requested a review from tbittar March 22, 2024 16:32

ianmnz added 4 commits May 30, 2024 14:13

Write subproblem with tree

393d39a

Add first pathway test

2171b31

Add three node test for pathway

a76b96b

Add prob to tree nodes for expectation computation

1e51b36

ianmnz marked this pull request as ready for review June 10, 2024 10:45

ianmnz changed the title ~~Draft - Feature/network cloning~~ Feature/network cloning Jun 10, 2024

Remove test debug init

7893e94

ianmnz requested a review from sylvlecl June 21, 2024 12:10

ianmnz added 2 commits July 11, 2024 13:19

Add prob to decision tree node

0f50a9b

Add ExpectedValueStrategy to Benders

9e92f54

tbittar requested changes Jul 12, 2024

View reviewed changes

Retour of PR comments

244fa11

tbittar approved these changes Jul 22, 2024

View reviewed changes

ianmnz merged commit 964297a into feature/investment_pathway Jul 22, 2024
1 check passed

ianmnz deleted the feature/network_cloning branch July 22, 2024 12:48


		But here as we solve on the tree, we need to find the best solution in expectation on the set of paths in the tree.

		Case 1 : prob \| investment \| operational

Feature/network cloning #28

Feature/network cloning #28

Uh oh!

Conversation

ianmnz commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tbittar commented Jul 12, 2024

Uh oh!

ianmnz commented Jul 16, 2024

Uh oh!

Uh oh!

Uh oh!

ianmnz commented Mar 22, 2024 •

edited

Loading