-
Notifications
You must be signed in to change notification settings - Fork 313
Bidding zones representation + custom busmap #1578
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
Conversation
for more information, see https://pre-commit.ci
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.
Thank you @FabianHofmann! I added some comments. I haven't had the chance to review cluster_network.py
yet.
Co-authored-by: Thomas Gilon <thomas.gilon@openenergytransition.org>
for more information, see https://pre-commit.ci
fix: return type in build_bidding_zones
When creating the administrative shapes from the bidding zones ( Activating the Original bidding zones from Electricity MapsZone SE213 triggering the switchBidding zone after
|
great @tgilon that you could track it down! this is totally fine |
@tgilon would you mind adding a release note and checking that configtables documentation (in doc/configtables/) is in line? |
Done, but not pushed yet! @FabianHofmann I haven't tested the |
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.
Looks mostly good,
Some documentation adjustments for clustering settings and release notes are yet to be included.
Main points:
- Infinite links in
copperplate_buses
- Bidding zone mapping based on points rather than Voronoi overlap
- Optionality of
build_bidding_zones
I'll give it a spin now.
scripts/cluster_network.py
Outdated
if mode == "administrative" and "bz" in params.administrative.values(): | ||
bidding_zones = gpd.read_file(snakemake.input.bidding_zones) | ||
zone_name = bidding_zones.set_index("zone_name").cross_country_zone | ||
zone_name = zone_name.where(zone_name.notnull(), zone_name.index) | ||
nc.buses["zone"] = zone_name | ||
copperplate_buses(nc, zone_name) |
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.
As far as I understood, this is for the case that you want one country with bidding zone representation and others in NUTSX, right? I didn't understand why this is not handled directly when creating the busmap
. Wouldn't the regular handling of the busmap
variable do everything you need?
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.
I'll make this option available for all the clustering modes. It's not handled in the busmap
because we need at least one node per country for the following steps of the workflow. This option enables it.
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.
And to answer the question, it's not specifically for mixed topologies. The main use will be to copperplate nodes to model market zones.
Map bidding zones to regions on a country-by-country basis, assigning each region | ||
to the bidding zone with which it has the largest overlap. If a region doesn't | ||
overlap with any bidding zone, it's assigned to the nearest one as a fallback. |
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.
This function can be simplified to a simple sjoin
on the substation coordinates. Similar to the NUTS clustering, we don't want to use the (aggregated) Voronoi shapes for anything. The regions associated with the clustered buses should match the original bidding zone shapes.
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.
I had the same thought however it was not working as I hoped. the problem is that the borders of bidding zones and nuts regions are not perfectly aligned. which means, that nearly all nuts regions that are at a border of a bidding zone always get assigned to the correct bidding zone and the neighbouring (due to a slight overlap). Roughly, a third of all nuts regions fall into this category. given that we would then need the fallback case anyway, I implemented directly with the intersection logic. performance wise this is no problem, it's still fast.
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.
What role do the NUTS regions play here? Couldn't you just look at what substations/buses in the base.nc
are within the bidding zone shapefiles? No need to involve NUTS shapes in this case, right?
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.
The idea discussed with @bobbyxng and @FabianHofmann was to "just" add the bidding zones as another aggregation level. This is what's added in build_shapes
, so that it can be processed by base_network
like any other aggregation level. This leads to this edge case of border alignments, that this function solves.
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.
…d add release note
Co-authored-by: Fabian Neumann <fabian.neumann@outlook.de>
Regarding the Italian case, I managed to reproduce a similar issue using a NUTS2 config. run:
name: "bidding-zone-4"
scenario:
clusters:
- adm
planning_horizons:
- 2050
clustering:
mode: administrative
administrative:
level: 2
copperplate_regions:
- [DE00, LUG1]
- [UKNI, IE00]
build_bidding_zones:
remove_islands: true
aggregate_to_tyndp: true
simplify_network:
remove_stubs: true
remove_stubs_across_borders: true In this case, the same strange shape appears in Italy. This can be explained by examining in the base network that we are using. Reference using your test config 1 Looking at the base network, we can see that there is a line running from Firenze to the south of Volterra. There is also a line running from Prato to the North of Tarquinia. These lines cross. Removing the stubs, we link the Voltera zone with the Arezzo zone (including Firenze). The same thing happens in southern Italy. One could think of solving this with |
I would recommend setting Here is the associated shapefile ( However, a different approach is required for #1578. I suggest changing the OSM data. Would it be possible to have a AC bus @bobbyxng on the island ? This is the raw grid. Another option would be to force the carrier for this bus using something like this: n.buses.loc["way/140248154", "carrier"] = "AC" I'm not a big fan of this one. However, we could hardcode it while we wait for an OSM fix. |
Hi Thomas! I will look into this. Sorry for the late reply, have been on conference/workshop trips the past two weeks. |
No problem, it always takes me a while to catch up with new reviews.
How much time do you need? Should I suggest a quick fix in the meantime (and merge the PR)? Or should we wait for your fix? |
I order to give this a boost (this has already taking quite some resources), I would suggest a quick fix for now with a dependency on the OSM version and try to get the fix into the next version of the OSM. |
Regarding my previous comment: I checked and got this mixed up with Mallorca. Gotland on the AC side only has 110 kV. I don't know what the best approach would be here, keeping a 110 kV AC bus would be the easiest and most correct, just need to make sure that it does not cause any issues downstream. I am looking into it now and will propose a solution. Maybe give me until Friday latest so I can make sure everything is fine before pushing a new dataset to Zenodo :) Is there anything else that you want me to focus on in particular? @tgilon @FabianHofmann |
@tgilon @FabianHofmann Thanks for the fixes, your proposed temporary hotfix regarding Gotland also works fine. I was just running the entire workflow again and noticed the workflow breaks further downstream in the sector-coupled runs in So to mitigate any of the issues, the easiest fix would be to rename to "UKXX" bidding zones to "GBXX". The alternative would be to adapt all pypsa-eur scripts that rely on this substring mapping. |
I added a small condition in |
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.
finished reviewing and testing the PR.
Only open points form my side:
- "GB" vs. "UK" substring issue in
build_biomass_potentials
Thanks @bobbyxng! I'm currently testing my fix. This has resulted in some other collateral damage. |
…rd rather than tyndp
…ross_borders (Shetland Islands)
@bobbyxng Thank you for the review, suggestion and proactivity! I've fixed the naming convention. This now means that we are no longer following the bidding zone naming convention of the TYNDP 2024. The only exceptions are To ensure everything runs smoothly, I applied same type of fix for the Shetland Islands bus as we did for Öland. Could you please fix this in the next OSM prebuilt? The workflow runs entirely on my side using |
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.
I'm fine with this version!
I would not have mixed the bidding zone clustering with the administrative shapes, and would have overwritten the aggregated Voronoi regions with the original bidding zone shapefiles after a plain .sjoin
operation on the substations.
But this is also functional, and it's generally very useful to have a bidding zone level representation as an option.
remove_stubs_across_borders
should indeed be false.- I have amended the test config with the renaming of UKNI to GBNI.
Ready to merge?
Thank you @fneum for your review!
I left it unspecified so that we could use the default configuration, which is what we need. Completely fine for me to be specific, as this protects the test configuration file against further changes.
Good catch, thanks! Nothing else to add on my side. |
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.
also ready from my side! Thanks @tgilon and @FabianHofmann :)
The following changes add an additional layer for bidding zones on the administrative shapes introduced in #1502.
On top, a cluster mode
custom_busshapes
was added to support passing user-defined busshapes (instead of custom busmap) which is used to perform the clustering.Note that this is two distinct features.
TODO
@bobbyxng @tgilon if you already want to take a look feel free.
Checklist
envs/environment.yaml
.config/config.default.yaml
.doc/configtables/*.csv
.doc/data_sources.rst
.doc/release_notes.rst
is added.