-
Notifications
You must be signed in to change notification settings - Fork 212
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
Lossy Bidirectional Links #1192
Changes from 22 commits
ea57cea
9acbfe4
a1f91e4
31c7973
a0c0a05
a2150d6
1a98c9f
ac90aec
a3bd91d
80aee27
45fdc2b
77f557f
e38b221
fefe70b
4738a3f
e108be5
9f55920
2ac8db4
02fc071
9d14ac2
f6aa253
ca1db0a
7c87a48
4092176
fcb9911
007e427
b1ef404
090eeb9
d84ce66
eaba5cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -171,7 +171,7 @@ electricity: | |||||||||
Generator: [solar, onwind, offwind-ac, offwind-dc, OCGT] | ||||||||||
StorageUnit: [] # battery, H2 | ||||||||||
Store: [battery, H2] | ||||||||||
Link: [] # H2 pipeline | ||||||||||
Link: [H2 pipeline] # H2 pipeline | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for better understanding: what is a particular idea of changing the default configuration parameters? Agree that it may it be a good idea to make sure that the improvements introduced by this PR are being tested. It can be done by adding pypsa-earth/test/config.sector.yaml Lines 21 to 24 in ef525ee
What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes testing was the main reason for adding it. I've moved it to test/config.sector.yaml. |
||||||||||
|
||||||||||
powerplants_filter: (DateOut >= 2022 or DateOut != DateOut) | ||||||||||
custom_powerplants: false # "false" use only powerplantmatching (ppm) data, "merge" combines ppm and custom powerplants, "replace" use only custom powerplants | ||||||||||
|
@@ -603,6 +603,9 @@ sector: | |||||||||
transmission_efficiency: | ||||||||||
electricity distribution grid: | ||||||||||
efficiency_static: 0.97 # efficiency of distribution grid (i.e. 3% loses) | ||||||||||
H2 pipeline: | ||||||||||
efficiency_per_1000km: 1 | ||||||||||
compression_per_1000km: 0.017 # DEA technology data. Mean of Energy losses, lines 5000-20000 MW and lines >20000 MW for 2020, 2030 and 2050, [%/1000 km] | ||||||||||
|
||||||||||
dynamic_transport: | ||||||||||
enable: false # If "True", then the BEV and FCEV shares are obtained depending on the "Co2L"-wildcard (e.g. "Co2L0.70: 0.10"). If "False", then the shares are obtained depending on the "demand" wildcard and "planning_horizons" wildcard as listed below (e.g. "DF_2050: 0.08") | ||||||||||
|
@@ -696,7 +699,6 @@ sector: | |||||||||
biomass: biomass | ||||||||||
keep_existing_capacities: true | ||||||||||
|
||||||||||
|
||||||||||
solving: | ||||||||||
options: | ||||||||||
formulation: kirchhoff | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,7 +57,13 @@ | |
import numpy as np | ||
import pandas as pd | ||
import pypsa | ||
from _helpers import configure_logging, create_logger | ||
from _helpers import ( | ||
configure_logging, | ||
create_logger, | ||
lossy_bidirectional_links, | ||
override_component_attrs, | ||
set_length_based_efficiency, | ||
) | ||
from add_electricity import ( | ||
_add_missing_carriers_from_costs, | ||
add_nice_carrier_names, | ||
|
@@ -261,10 +267,15 @@ def attach_hydrogen_pipelines(n, costs, config): | |
p_nom_extendable=True, | ||
length=h2_links.length.values, | ||
capital_cost=costs.at["H2 pipeline", "capital_cost"] * h2_links.length, | ||
efficiency=costs.at["H2 pipeline", "efficiency"], | ||
carrier="H2 pipeline", | ||
Eddy-JV marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
# split the pipeline into two unidirectional links to properly apply transmission losses in both directions. | ||
lossy_bidirectional_links(n, "H2 pipeline") | ||
Eddy-JV marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# set the pipelines efficiency and the electricity required by the pipeline for compression | ||
set_length_based_efficiency(n, "H2 pipeline", " H2", config) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Eric-Nitschke please add a variable like and in the snakefile please add to the rule add_extra_components:
in order to trigger the rule in case the efficiencies were changed in the config file by the user. |
||
|
||
|
||
if __name__ == "__main__": | ||
if "snakemake" not in globals(): | ||
Eddy-JV marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -274,7 +285,8 @@ def attach_hydrogen_pipelines(n, costs, config): | |
|
||
configure_logging(snakemake) | ||
|
||
n = pypsa.Network(snakemake.input.network) | ||
overrides = override_component_attrs(snakemake.input.overrides) | ||
n = pypsa.Network(snakemake.input.network, override_component_attrs=overrides) | ||
Nyears = n.snapshot_weightings.objective.sum() / 8760.0 | ||
config = snakemake.config | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be sure I get this point properly: is shadow directory the one which is created by the solver to store the temporary files? I wonder if this behaviour may also depend on a solver...
Regarding the files management, it has been found previously that
os.getcwd( )
may lead to some troubles if pypsa-earth model is being used as snakemake submodule. In particular #1137 has fixed this replacingos.getcwd( )
by a custom directory path. May it be a way to go also in our case?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In short:
The shadow directory is a Snakemake thing and not solver related.
With the "shadow" rule Snakemake creates an isolated temporary directory, in which it runs the corresponding rule (in this case "solve_network"). With
shadow: "shallow"
, Snakemake symlinks all top level files and directory so that all relative file paths provided will still reach the right directories. However withshadow: "copy-minimal"
, the inputs of the rule are copied instead of symlinked. In my tests this led to Snakemake not having access to the files within the "data/override_component_attrs" folder. My guess is, that copy minimal only copies files directly, but not the content of directories. PR #790 is the reason, why "copy-minimal" is needed on Windows.You can check the Snakemake documentation for more details, though it is not that extensive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, I think I found the issue:
else "/data/override_component_attrs"
should beelse "data/override_component_attrs"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, itmight be nice to move away from
os.getcwd()
to the changes in #1137. However, I am not confident in implementing these changes myself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davide-f This definitely needs your attention as you already made PR #790. And @Eric-Nitschke if this approved by Davide, then probably the solution should be done for all Rules that use the solve_network.py script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os.getcwd() must be avoided as it does not track the use of submodules and can lead to issues.
Have you tried with directory(data/...)? [although the directory command should be optional for inputs].
Moreover, not sure if copy-minimal is strictly needed here; have you tested?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davide-f I've tried running it on Windows with "shallow" instead, which led to issues with admin rights similar to PR #790.
I've not tried directory(data/...) yet. Would that look something like this?
Removing the different handling (as done by @Eddy-JV) will most likely mean, that the code will not work properly on Windows since it will not find the proper files when running with "copy-minimal" (an issue that will not be solved by PR #1295).
However, I'm in favor of doing it this way, if it means that we'll merge this PR somewhat soon and create a new issue for the Windows issues.