feat: restructure and improve transmission topology + config for all carriers#2153
feat: restructure and improve transmission topology + config for all carriers#2153bobbyxng wants to merge 29 commits into
Conversation
…itial build_transmission script using Delaunay and Gabriel filtering.
|
Both new methods yield roughly the same ballpark wrt. number of edges in a 200-node network: Applying the Gabriel filter returns the minimum number of edges obtained through this method. The minimum degree of nodes is set to 1. By changing the degree, the topology can be increased again. So we are always moving in the range If you wanna get rid of "stubs", you can simply set the minimum degree to 2 or higher |
…) to transmission key.
|
I consolidated old and new keys into the setup below and updated the associated doc entries as well as config examples (please double-check) @fneum I was not sure how to handle these settings, which are partly related to the transmission infrastructure in
Maybe some of those keys can also be moved to the respective transmission carrier, happy to receive your feedback here @fneum |
|
|
…nstead of carriers.
…rojects_and_dlr and build_transmission projects for clarity.
|
Added a key that allows to set a maximum offshore (haversine) distance, similar to min_degree. This gives a bit more fine-grained control for offshore connections that the user deems unreasonable upfront Here's an example with an arbitrary maximum haversine distance of 300 km offshore |
…ions [][efficiency] respectively.
…the gas distribution grid, only capital costs. Clean-up.
|
I believe I finalised everything now. All transmission related settings are consolidated in the new New setup: |
|
Ready for (final) review @fneum @brynpickering |
brynpickering
left a comment
brynpickering
left a comment
There was a problem hiding this comment.
Thanks for the updates @bobbyxng . As you might notice, I'm not really reviewing your implementation. I trust that it works as you expect it to. Instead, I'm concerned with maintainability and user-facing (i.e. config). @fneum might be more qualified to actually dissect the delaunay and filtering methods
| max_offshore_haversine_distance: float = Field( | ||
| float("inf"), | ||
| gt=0, | ||
| description="Maximum haversine distance in km for offshore transmission candidates. Candidate edges a total offshore length exceeding this threshold are excluded. Defaults to infinity (no limit).", | ||
| ) |
There was a problem hiding this comment.
move to within gabriel_filter, perhaps by renaming to filter_edges and having gabriel_filter_min_degree and max_offshore_haversine_distance within it (since the max_offshore distance is currently only being triggered when the gabriel filter is also active).
There was a problem hiding this comment.
Setting gabriel_filter_min_degree default to zero will effectively switch it off, so if a user wants to activate it, they can set it to >= 1. Then you have the enable key which defines whether filtering should be attempted and then two keys that default to effectively not being applied unless a user changes the values (< inf for haversine, > 0 for gabriel).
This is arguably overkill, you could even avoid the enable key since they'd default to not being applied anyway, but this approach leaves open the option to add further filtering methods in future and switch them on/off as a group as desired.
There was a problem hiding this comment.
Good point, I have also changed the config now so the user can choose to limit the candidates based on max_offshore_haversine_distance without using the gabriel filter.
Example
carbon_dioxide:
enable: true
gabriel_filter_min_degree: 0
max_offshore_haversine_distance: 700 # km
| for carrier, carrier_cfg in config.get("transmission", {}).items(): | ||
| if not isinstance(carrier_cfg, dict) or not carrier_cfg.get("enable", False): | ||
| continue | ||
| gabriel_cfg = carrier_cfg.get("gabriel_filter") | ||
| if not isinstance(gabriel_cfg, dict): | ||
| continue | ||
| key = f"{carrier}_transmission_candidates" | ||
| if gabriel_cfg.get("enable", False) and "min_degree" in gabriel_cfg: | ||
| min_degree = int(gabriel_cfg["min_degree"]) | ||
| max_offdist = f"{float(carrier_cfg.get('max_offshore_haversine_distance', float('inf'))):g}" | ||
| candidates[key] = resources( | ||
| f"transmission/candidates_{{clusters}}_min_{min_degree}_maxoffdist_{max_offdist}_km.geojson" | ||
| ) | ||
| else: | ||
| candidates[key] = resources("transmission/all_edges_{clusters}.geojson") | ||
| return candidates |
There was a problem hiding this comment.
This is overly cautious given what is being enforced in by the config validation. Transmission must be a dict with all carrier keys defined. min_degree and max_offshore will also be int/float respectively as that is forced by the validator.
| for carrier, carrier_cfg in config.get("transmission", {}).items(): | |
| if not isinstance(carrier_cfg, dict) or not carrier_cfg.get("enable", False): | |
| continue | |
| gabriel_cfg = carrier_cfg.get("gabriel_filter") | |
| if not isinstance(gabriel_cfg, dict): | |
| continue | |
| key = f"{carrier}_transmission_candidates" | |
| if gabriel_cfg.get("enable", False) and "min_degree" in gabriel_cfg: | |
| min_degree = int(gabriel_cfg["min_degree"]) | |
| max_offdist = f"{float(carrier_cfg.get('max_offshore_haversine_distance', float('inf'))):g}" | |
| candidates[key] = resources( | |
| f"transmission/candidates_{{clusters}}_min_{min_degree}_maxoffdist_{max_offdist}_km.geojson" | |
| ) | |
| else: | |
| candidates[key] = resources("transmission/all_edges_{clusters}.geojson") | |
| return candidates | |
| for carrier, carrier_cfg in config["transmission"].items(): | |
| gabriel_cfg = carrier_cfg.get("gabriel_filter") | |
| if ("gabriel_filter" := carrier_cfg.get("gabriel_filter")) is None or not carrier_cfg["enable"]: | |
| continue | |
| key = f"{carrier}_transmission_candidates" | |
| if gabriel_cfg["enable"]: | |
| min_degree = gabriel_cfg["min_degree"] | |
| max_offdist = carrier_cfg["max_offshore_haversine_distance"] | |
| candidates[key] = resources( | |
| f"transmission/candidates_{{clusters}}_min_{min_degree}_maxoffdist_{max_offdist}_km.geojson" | |
| ) | |
| else: | |
| candidates[key] = resources("transmission/all_edges_{clusters}.geojson") | |
| return candidates |
There was a problem hiding this comment.
btw, I don't like that the existence of the gabriel_filter key implicitly defines that the network will be defined by delaunay triangulation. That is, there's nothing explicit in the config to suggest how the edges will be generated, except for electricity which has base_network. Could we perhaps have base_network as a config option for every transmission carrier, then limit it to delaunay for hydrogen and co2, osm(?) for gas. Would leave open the option to extend the base network options in future (e.g. using only existing gas network for h2)
There was a problem hiding this comment.
Implemented the first comment.
Regarding the second, had similar thoughts on this, however, without a clear conclusion yet on this:
- @fneum suggested to drop the "legacy" option to generate alternative topologies for H2 and CO2, i.e., from the electricity high voltage grid (there is no really good reason keeping the old approach)
- If we were to include
base_network, we could either limit the options todelaunayfortransmission.hydrogen.base_networkandtransmission.carbon_dioxide.base_networkor also allowelectricity_topologyif we still want to enable the original behaviour of the current master branch - Regarding using only existing gas network for H2, there is some overlap with using transmission.hydrogen.retrofit, which does not fully mean the same thing as what you are suggesting, might introduce a bit of confusion though.
- I generally like the idea to have
base_networkfor each transmission carrier, even if for now, the options would be limited.
There was a problem hiding this comment.
I generally like the idea to have base_network for each transmission carrier, even if for now, the options would be limited.
Yeah, I'd also be fine with it even if it just has delaunay for now (or also the electricity_topology for backwards-compatibility, but then you need the logic to pull that topology when requested, so maybe not worth it).
There was a problem hiding this comment.
Regarding using only existing gas network for H2, there is some overlap with using transmission.hydrogen.retrofit, which does not fully mean the same thing as what you are suggesting, might introduce a bit of confusion though.
Yeah, true. It would be neat to be able to define base_network: gas_topology and then retrofit could be used to define whether it can use the existing base_network or has to build from scratch following the same topology. Maybe not something for now.
| buses_prev, lines_prev, links_prev = len(n.buses), len(n.lines), len(n.links) | ||
|
|
||
| linetype_380 = snakemake.config["lines"]["types"][380] | ||
| linetype_380 = snakemake.config["transmission"]["electricity"]["lines"]["types"][ |
There was a problem hiding this comment.
This is the opposite of my earlier comment: there's no requirement from the validator for the user to define the 380kV line type. They could replace the line types dict and not include this level. This call will then fail. Since it's a critical level, I would add a field validator to the validator to ensure the 380 key is always defined (perhaps beyond the scope of this PR, open an issue if so).
2 participants
Closes #2151
Changes proposed in this Pull Request
carrierwildcard supportbuild_transmission.pyconfiguration.rstMotivation
Checklist
Required:
doc/release_notes.rst.If applicable:
scripts/lib/validation.doc/*.rstfiles.