fix to avoid negative objective value #961

GbotemiB · 2024-01-18T17:39:07Z

Closes #956

Changes proposed in this Pull Request

The changes in the PR is as suggested in #956 which is to adjust the lines in base_network.py:_set_lines_s_nom_from_linetypes from s_nom to s_nom_min to prevent negative objective values after solving network.

Checklist

I consent to the release of this PR's code under the AGPLv3 license and non-code contributions under CC0-1.0 and CC-BY-4.0.
I tested my contribution locally and it seems to work fine.
Code and workflow changes are sufficiently documented.
Newly introduced dependencies are added to envs/environment.yaml and doc/requirements.txt.
Changes in configuration options are added in all of config.default.yaml and config.tutorial.yaml.
Add a test config or line additions to test/ (note tests are changing the config.tutorial.yaml)
Changes in configuration options are also documented in doc/configtables/*.csv and line references are adjusted in doc/configuration.rst and doc/tutorial.rst.
A note for the release notes doc/release_notes.rst is amended in the format of previous release notes, including reference to the requested PR.

davide-f

Great @GbotemiB :)

So, the change should not be placed here, but rather in prepare_network.
By looking at the code in prepare_network, we actually have a function "set_transmission_limits" that should do that, but it seems it is not working.

pypsa-earth/scripts/prepare_network.py

Line 188 in e4edb17

def set_transmission_limit(n, ll_type, factor, costs, Nyears=1):

It may be good to investigate why, may you be interested in that?

GbotemiB · 2024-01-20T02:05:49Z

Thanks @davide-f,

However, when I changed s_nom to s_nom_min in base_network.py, it worked to prevent my networks from having negative objective values.

I will run the prepare_network with a debug. Hopefully, I will be able to understand why it didn't work in the first instance.

davide-f · 2024-01-20T17:52:51Z

Thanks @davide-f,

However, when I changed s_nom to s_nom_min in base_network.py, it worked to prevent my networks from having negative objective values.

I will run the prepare_network with a debug. Hopefully, I will be able to understand why it didn't work in the first instance.

It works, but if that option is changed there, there can be problems if the user desire to not do capacity expansion on the network as p_nom doesnt get initialized properly.
The set_transmission limit was meant to do that but it seems something is not working well

davide-f · 2024-02-05T12:49:37Z

@GbotemiB not sure if you are also working here :) In case of problems, needs feel free to write :)

GbotemiB · 2024-02-05T13:22:59Z

@GbotemiB not sure if you are also working here :) In case of problems, needs feel free to write :)

Thanks @davide-f, I looked into it last week, I couldn't fix it yet. But I will report progress on this later today.

GbotemiB · 2024-02-08T00:44:14Z

@davide-f I have been able to finally debug the prepare_network.py

pypsa-earth/scripts/prepare_network.py

Line 188 in e4edb17

def set_transmission_limit(n, ll_type, factor, costs, Nyears=1):

It turns out the section of the highlighted code below doesn't get executed.

pypsa-earth/scripts/prepare_network.py

Lines 207 to 212 in e4edb17

    
           if factor == "opt" or float(factor) > 1.0: 
        
               n.lines["s_nom_min"] = lines_s_nom 
        
               n.lines["s_nom_extendable"] = True 
        
               n.links.loc[links_dc_b, "p_nom_min"] = n.links.loc[links_dc_b, "p_nom"] 
        
               n.links.loc[links_dc_b, "p_nom_extendable"] = True

The following image should give more context into why the conditional is not working.

GbotemiB · 2024-02-08T19:14:34Z

pypsa-earth/scripts/prepare_network.py

Line 197 in e4edb17

lines_s_nom = n.lines.s_nom.where(n.lines.type == "", _lines_s_nom)

@davide-f, If you don't mind, can you clarify the purpose of this section

GbotemiB · 2024-02-08T19:18:49Z

pypsa-earth/scripts/prepare_network.py

Lines 191 to 196 in e4edb17

    
           _lines_s_nom = ( 
        
               np.sqrt(3) 
        
               * n.lines.type.map(n.line_types.i_nom) 
        
               * n.lines.num_parallel 
        
               * n.lines.bus0.map(n.buses.v_nom) 
        
           )

I also noticed that num_parallel here is a little lower than the same that was used in base_network.py

davide-f · 2024-02-11T11:15:36Z

@davide-f I have been able to finally debug the prepare_network.py

pypsa-earth/scripts/prepare_network.py

Line 188 in e4edb17

def set_transmission_limit(n, ll_type, factor, costs, Nyears=1):

It turns out the section of the highlighted code below doesn't get executed.

pypsa-earth/scripts/prepare_network.py

Lines 207 to 212 in e4edb17

if factor == "opt" or float(factor) > 1.0:

n.lines["s_nom_min"] = lines_s_nom

n.lines["s_nom_extendable"] = True

n.links.loc[links_dc_b, "p_nom_min"] = n.links.loc[links_dc_b, "p_nom"]

n.links.loc[links_dc_b, "p_nom_extendable"] = True

The following image should give more context into why the conditional is not working.

Nice catch!
I believe it makes sense to revise the default parameters of tutorial and default to make sure that works.
Probably, the v0.3 should be revised in to opt (maybe better) or v1.5 (like in pypsa-eur) or something like that.

Great :D

davide-f · 2024-02-11T11:18:36Z

pypsa-earth/scripts/prepare_network.py

Lines 191 to 196 in e4edb17

_lines_s_nom = (

np.sqrt(3)

* n.lines.type.map(n.line_types.i_nom)

* n.lines.num_parallel

* n.lines.bus0.map(n.buses.v_nom)

)

I also noticed that num_parallel here is a little lower than the same that was used in base_network.py

This was imported from pypsa-eur.
The idea is that where the lines do not have a specific line type (which corresponds to a s_nom), the s_nom is derived from current and voltage of that line.
I guess the idea behind it is to fill only those values that are not filled yet.
Did you find issues with this?

davide-f · 2024-02-11T11:19:21Z

pypsa-earth/scripts/prepare_network.py

Lines 191 to 196 in e4edb17

_lines_s_nom = (

np.sqrt(3)

* n.lines.type.map(n.line_types.i_nom)

* n.lines.num_parallel

* n.lines.bus0.map(n.buses.v_nom)

)

I also noticed that num_parallel here is a little lower than the same that was used in base_network.py

This is instead expected as in the simplify_network phase, the whole network is simplified to a base voltage (e.g. 380kV) equivalent and we play with num_parallel to handle the equivalence.
See

pypsa-earth/scripts/simplify_network.py

Line 118 in e4edb17

def simplify_network_to_base_voltage(n, linetype, base_voltage):

GbotemiB · 2024-02-14T12:53:07Z

I went through simplify_network.py. Why is there a need to recalculate num_parallel

pypsa-earth/scripts/simplify_network.py

Line 135 in e4edb17

n.lines["num_parallel"] = n.lines.eval("s_nom / (sqrt(3) * v_nom * i_nom)")

GbotemiB · 2024-02-14T12:56:37Z

Also, I noticed that in base_network.py, the line types is as in the image

while in prepare_network.py, the line types is

I think it might have to do with the rebasing of voltages

GbotemiB · 2024-02-14T13:05:59Z

I also found out that n.lines.s_nom and _lines_s_nom have different values in prepare_network.py

when I used replace lines_s_nom by n.lines.s_nom here, my solved network didn't have any negative objectives

pypsa-earth/scripts/prepare_network.py

Lines 207 to 212 in e4edb17

    
           if factor == "opt" or float(factor) > 1.0: 
        
               n.lines["s_nom_min"] = lines_s_nom 
        
               n.lines["s_nom_extendable"] = True 
        
               n.links.loc[links_dc_b, "p_nom_min"] = n.links.loc[links_dc_b, "p_nom"] 
        
               n.links.loc[links_dc_b, "p_nom_extendable"] = True

GbotemiB · 2024-02-14T13:06:08Z

looking at v_nom, the values are 300kV, which I don't think we should be doing the multiplication with line_types of 380kV

GbotemiB · 2024-02-20T10:35:50Z

Hi @davide-f, It looks like the reason why n.lines.s_nom is different from _lines_s_nom is because the parameters used to calculate _lines_s_nom has been altered in simplify_network:simplify_network_to_base_voltage

pypsa-earth/scripts/simplify_network.py

Lines 129 to 135 in 9168ca7

    
           logger.info(f"Mapping all network lines onto a single {int(base_voltage)}kV layer") 
        
           n.lines["type"] = linetype 
        
           n.lines["v_nom"] = base_voltage 
        
           n.lines["i_nom"] = n.line_types.i_nom[linetype] 
        
           # Note: s_nom is set in base_network 
        
           n.lines["num_parallel"] = n.lines.eval("s_nom / (sqrt(3) * v_nom * i_nom)")

GbotemiB · 2024-02-20T10:47:17Z

Regarding p_nom here

pypsa-earth/scripts/prepare_network.py

Lines 211 to 212 in e4edb17

    
           n.links.loc[links_dc_b, "p_nom_min"] = n.links.loc[links_dc_b, "p_nom"] 
        
           n.links.loc[links_dc_b, "p_nom_extendable"] = True

During debugging, n.links.p_nom and n.links.p_nom_min results to zero.

But p_nom and p_nom_min in generators are the same which is not equal to zero.

ekatef · 2024-02-20T11:20:22Z

Hello!

@GbotemiB it looks like you have done a quite intense investigation and found very nice bugs (in particular, the one which relates to loading the options from the file name looks great!!), However, the whole picture still looks a bit too complicated. I'd suggest to take a step back and clarify the architecture of the lines transformations workflow, starting from osm-extracted data up to the pre-solved network. I'll doing something similar at the moment and happy to support you with that.

The resulted diagram would facilitate discussion of high-level features with @davide-f

Once the intended path would have been clear, it will simplify testing and introducing fixes, as it'll be clear what should we expect at each stage.

What do you think @GbotemiB?

GbotemiB · 2024-02-20T12:06:46Z

Thanks @ekatef.

You are definitely right. I will start with clarifying the workflow.

GbotemiB · 2024-05-23T14:46:18Z

Hello @davide-f @ekatef @pz-max

Following last week's discussion on a possible check for the negative objective issue.

I did a check to verify the results from the solved networks (Monte-Carlo Runs) p_nom_opt - p_nom to see if p_nom_opt > p_nom

davide-f · 2024-05-29T08:47:55Z

Ok, the problem is in lines and can you confirm that the following code is executed?

pypsa-earth/scripts/prepare_network.py

Lines 206 to 211 in de4da14

    
           if factor == "opt" or float(factor) > 1.0: 
        
               n.lines["s_nom_min"] = lines_s_nom 
        
               n.lines["s_nom_extendable"] = True 
        
               n.links.loc[links_dc_b, "p_nom_min"] = n.links.loc[links_dc_b, "p_nom"] 
        
               n.links.loc[links_dc_b, "p_nom_extendable"] = True

davide-f · 2024-05-29T09:04:02Z

Moreover:

please check in pypsa-eur if in base_network the s_nom_min is also specified
please check in pypsa-eur the same chunk of code in the equivalent script to check how that works there and see if there are major differences

Many thanks!

GbotemiB · 2024-05-30T21:47:45Z

@davide-f

Checking pypsa-eur prepare_network.py

https://github.com/PyPSA/pypsa-eur/blob/4fbb3c81c4ddb589614f67dc01c885c9b3806bbe/scripts/prepare_network.py#L78-L97

This is the part that has to do with lines that is present in PyPSA-Eur but not in PyPSA-Earth

GbotemiB · 2024-05-30T21:53:02Z

Ok, the problem is in lines and can you confirm that the following code is executed?

pypsa-earth/scripts/prepare_network.py

Lines 206 to 211 in de4da14

if factor == "opt" or float(factor) > 1.0:

n.lines["s_nom_min"] = lines_s_nom

n.lines["s_nom_extendable"] = True

n.links.loc[links_dc_b, "p_nom_min"] = n.links.loc[links_dc_b, "p_nom"]

n.links.loc[links_dc_b, "p_nom_extendable"] = True

I also already confirm that this part is executed in the workflow.

please check in pypsa-eur if in base_network the s_nom_min is also specified

Also in pypsa-eur, s_nom_min is not specified in base_network.

davide-f · 2024-06-05T23:14:07Z

Ok, so @GbotemiB I understand that the problem is back about why the s_nom_min calculated here differs from the p_nom value available in the network right?
It seems that one of the causes seems due to the missing name in the line type; did you get why that happens?
My feeling is that it is related to the clustering procedure

GbotemiB · 2024-06-06T01:30:53Z

Hi @davide-f, I haven't figured out the cause of the missing names in the line type. I will investigate the clustering procedure to see if something has been missed there.

GbotemiB · 2024-06-10T12:57:23Z

Hi @davide-f, I have just uploaded a simple notebook here. I made some checks in the notebook. Feel free to look through it.

davide-f · 2024-06-11T18:06:34Z

Interesting @GbotemiB ! We are getting closer to the problem!
I see you compare: p_nom.sum(), p_nom_min.sum()
Could you add to the list also the following computation:
n.lines.eval("sqrt(3) * i_nom * v_nom") ?
Note that i_nom may not be available for the line and there may be the need to be retrieved similarly to the code you have checked some comments previously.
I think we should identify when differences occur and solve the issue.
Note, the sum may not be the best approach but for now it does ok

GbotemiB · 2024-06-14T07:35:38Z

Interesting @GbotemiB ! We are getting closer to the problem! I see you compare: p_nom.sum(), p_nom_min.sum() Could you add to the list also the following computation: n.lines.eval("sqrt(3) * i_nom * v_nom") ? Note that i_nom may not be available for the line and there may be the need to be retrieved similarly to the code you have checked some comments previously. I think we should identify when differences occur and solve the issue. Note, the sum may not be the best approach but for now it does ok

Just a little clarification. The difference section was between p_nom_opt and p_nom, while the other was between s_nom and s_nom_min.

I wanted to observe the changes in the workflow for s_nom and s_nom_min.

GbotemiB · 2024-06-14T10:23:28Z

I have updated the notebook here. I also rearranged the process for ease of understanding.

I also noticed that num_parallel was not included in the calculation formula. Was it intentional?

davide-f · 2024-07-01T15:21:48Z

I have updated the notebook here. I also rearranged the process for ease of understanding.

I also noticed that num_parallel was not included in the calculation formula. Was it intentional?

Cool Emmanuel, apologies missed communication.
Could you update the last cells of the notebook to account for the number of parallel lines?
With:

(np.sqrt(3)
	* base.lines["type"].map(base.line_types.i_nom) 
	* base.lines.v_nom
	* base.lines.num_parallel).sum()

We are moving forward! 🚀 :D

GbotemiB · 2024-07-02T06:31:27Z

Thanks @davide-f,

I have updated the cells in the notebook.
Feel free to check again.

davide-f · 2024-07-03T23:59:46Z

Amazing progress @GbotemiB ! :D :D
We identified the script where problem occurs: simplify_network
Indeed for that network, the calculation using the product i_nomv_nomnum_parallel * sqrt(3) does not match the s_nom value.

Somewhere there some calculations are performed.
The huntch is that during the various steps of the simplification and probably the aggregation strategies, something does not go as expected.

To debug that, I'd create a simple auxiliary function that computes the i_nomv_nomnum_parallel product, reads the s_nom value and check if the values match and print the results.

I'd test that for the various modified functions across the script to identify where the occurrence arrives.

What do you think?

GbotemiB · 2024-07-04T07:50:36Z

@davide-f, I'm glad we identified the simplify_network script as the source of the issue.
I'll go ahead and implement this function and test it at different points in the script. Hopefully, this will help us pinpoint exactly where things are going wrong.

davide-f · 2024-07-04T22:16:44Z

Great :D Thank you!!!
As heads up, my guess is that during some aggregation strategies, line types are lost and/or the num_parallel are not summed up.

GbotemiB · 2024-07-08T10:05:50Z

Hello @davide-f,

I looked over the simplify_network.py script, looks like there is an additional line present in pypsa-eur that is not in pypsa-earth. You can check here

I added the line to my local files in the script:simplify_network_to_base_voltage.py, this seems to resolved the negative objective issue.

What are your thoughts?

GbotemiB · 2024-07-08T13:52:22Z

I have also updated the notebook here with the recent changes I made.

davide-f · 2024-07-10T21:55:38Z

Hello @davide-f,

I looked over the simplify_network.py script, looks like there is an additional line present in pypsa-eur that is not in pypsa-earth. You can check here

I added the line to my local files in the script:simplify_network_to_base_voltage.py, this seems to resolved the negative objective issue.

What are your thoughts?

Really?

Yeah, definitely that should be added, however, instead of having "380" fixed, it should be base_voltage.
Does that really fix the issue? Does the notebook consider this?

It seems a too easy fix... XD

Update: I've better checked the script but unfortunately that does not solve the issue.
In the notebook, as a comment it is difficult to compare the values: we need to scroll a lot to compare the s_nom value and the corresponding s_nom computed using the formula.

By comparing the values, for example, for siml,
the sum of s_noms is: 327228
while the corresponding value computed using sqrt(3) * i_nom * v_nom * num_parallel is: 281636
The two values should match

GbotemiB · 2024-07-11T00:54:52Z

@davide-f, I will check the notebook again. But I tested the implementation against the monte-carlo notebook. The negative objective values seem to not be present.
https://github.com/GbotemiB/documentation/blob/monte-carlo/notebooks/monte_carlo_tutorial.ipynb

davide-f · 2024-07-11T09:49:29Z

@davide-f, I will check the notebook again. But I tested the implementation against the monte-carlo notebook. The negative objective values seem to not be present. https://github.com/GbotemiB/documentation/blob/monte-carlo/notebooks/monte_carlo_tutorial.ipynb

I understand, but the objective can still be misleading.
The s_nom_min should match s_nom.
Now we have reduced the difference but the problem is not fixed definitely unfortunately, if you had still resources. If not no problem

GbotemiB · 2024-07-11T15:26:53Z

@davide-f, I will check the notebook again. But I tested the implementation against the monte-carlo notebook. The negative objective values seem to not be present. https://github.com/GbotemiB/documentation/blob/monte-carlo/notebooks/monte_carlo_tutorial.ipynb

I understand, but the objective can still be misleading. The s_nom_min should match s_nom. Now we have reduced the difference but the problem is not fixed definitely unfortunately, if you had still resources. If not no problem

@davide-f, Looks like I forgot to update the path in the notebook. I think we can smile now 😄
I updated the runs already, feel free to check.
https://github.com/GbotemiB/csp-design/blob/notebook/negative_obj.ipynb

davide-f

Great @GbotemiB :D
Would you like to add a release note? :)
This PR is ready to fly! :)

davide-f · 2024-07-12T08:46:44Z

Many thanks @GbotemiB ! Merging :)

fix to avoid negative objective value

8de787b

davide-f reviewed Jan 18, 2024

View reviewed changes

Merge branch 'main' into s_nom_fix

2b9c9be

Merge branch 'main' into s_nom_fix

095a913

GbotemiB added 2 commits February 5, 2024 18:22

Merge branch 'main' into s_nom_fix

1530d61

Merge branch 'main' into s_nom_fix

25b4e43

Merge branch 'main' into s_nom_fix

0fc4fa8

Merge branch 'main' into s_nom_fix

79edc31

ekatef mentioned this pull request Mar 22, 2024

Monte carlo case study pypsa-meets-earth/documentation#78

Merged

GbotemiB added 2 commits May 15, 2024 11:27

Merge branch 'main' into s_nom_fix

4edab09

Merge branch 'pypsa-meets-earth:main' into s_nom_fix

e45980f

Merge branch 'pypsa-meets-earth:main' into s_nom_fix

5fc77d8

GbotemiB added 2 commits July 8, 2024 13:09

added rebase voltage for buses

b298ab4

Merge branch 'main' into s_nom_fix

401064f

Merge branch 'pypsa-meets-earth:main' into s_nom_fix

4385891

davide-f reviewed Jul 11, 2024

View reviewed changes

added release note

8de07d9

davide-f merged commit af64048 into pypsa-meets-earth:main Jul 12, 2024
5 checks passed

ekatef mentioned this pull request Dec 23, 2024

Linopy integration for Power and Sector-Coupled Modelling #1172

Open

8 tasks

fix to avoid negative objective value #961

fix to avoid negative objective value #961

Conversation

GbotemiB commented Jan 18, 2024 • edited Loading

Closes #956

Changes proposed in this Pull Request

Checklist

davide-f left a comment

Choose a reason for hiding this comment

GbotemiB commented Jan 20, 2024

davide-f commented Jan 20, 2024

davide-f commented Feb 5, 2024

GbotemiB commented Feb 5, 2024

GbotemiB commented Feb 8, 2024 • edited Loading

GbotemiB commented Feb 8, 2024 • edited Loading

GbotemiB commented Feb 8, 2024

davide-f commented Feb 11, 2024

davide-f commented Feb 11, 2024

davide-f commented Feb 11, 2024 • edited Loading

GbotemiB commented Feb 14, 2024 • edited Loading

GbotemiB commented Feb 14, 2024

GbotemiB commented Feb 14, 2024

GbotemiB commented Feb 14, 2024

GbotemiB commented Feb 20, 2024

GbotemiB commented Feb 20, 2024 • edited Loading

ekatef commented Feb 20, 2024

GbotemiB commented Feb 20, 2024

GbotemiB commented May 23, 2024

davide-f commented May 29, 2024

davide-f commented May 29, 2024 • edited Loading

GbotemiB commented May 30, 2024

GbotemiB commented May 30, 2024

davide-f commented Jun 5, 2024

GbotemiB commented Jun 6, 2024

GbotemiB commented Jun 10, 2024

davide-f commented Jun 11, 2024 • edited Loading

GbotemiB commented Jun 14, 2024

GbotemiB commented Jun 14, 2024 • edited Loading

davide-f commented Jul 1, 2024 • edited Loading

GbotemiB commented Jul 2, 2024

davide-f commented Jul 3, 2024 • edited Loading

GbotemiB commented Jul 4, 2024

davide-f commented Jul 4, 2024 • edited Loading

GbotemiB commented Jul 8, 2024

GbotemiB commented Jul 8, 2024

davide-f commented Jul 10, 2024 • edited Loading

GbotemiB commented Jul 11, 2024 • edited Loading

davide-f commented Jul 11, 2024 • edited Loading

GbotemiB commented Jul 11, 2024

davide-f left a comment

Choose a reason for hiding this comment

davide-f commented Jul 12, 2024

GbotemiB commented Jan 18, 2024 •

edited

Loading

GbotemiB commented Feb 8, 2024 •

edited

Loading

GbotemiB commented Feb 8, 2024 •

edited

Loading

davide-f commented Feb 11, 2024 •

edited

Loading

GbotemiB commented Feb 14, 2024 •

edited

Loading

GbotemiB commented Feb 20, 2024 •

edited

Loading

davide-f commented May 29, 2024 •

edited

Loading

davide-f commented Jun 11, 2024 •

edited

Loading

GbotemiB commented Jun 14, 2024 •

edited

Loading

davide-f commented Jul 1, 2024 •

edited

Loading

davide-f commented Jul 3, 2024 •

edited

Loading

davide-f commented Jul 4, 2024 •

edited

Loading

davide-f commented Jul 10, 2024 •

edited

Loading

GbotemiB commented Jul 11, 2024 •

edited

Loading

davide-f commented Jul 11, 2024 •

edited

Loading