Skip to content
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

introduce hierarchical agglomeratice clustering (hac) #293

Merged
merged 20 commits into from
Jun 28, 2022

Conversation

martacki
Copy link
Member

@martacki martacki commented Nov 17, 2021

Changes proposed in this Pull Request

...story continues...

Introduce hierarchical agglomeraticve clustering method into pypsa-eur, now that it is (finally) included in PyPSA PyPSA/PyPSA#289.

(additional) changes:

  1. simplify now by default aggregates buses that are no substations.
  2. clustering now defaults to the new method "hac", if not specified for kmeans.
  3. clustering method in simplify_network can be different from the method used in cluster_network.

Locally the function(s) worked fine.

Checklist

  • 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 envs/environment.docs.yaml.
  • Changes in configuration options are added in all of config.default.yaml, config.tutorial.yaml, and test/config.test1.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.

@fneum fneum self-requested a review November 29, 2021 08:39
Copy link
Member

@fneum fneum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

So HAC will be the new default? Could you highlight this in a release note? It's quite a significant change.

Two maps with clustered regions (~200 nodes) with previous k-means default and new HAC defaults would also very nicely complement this PR. Do you have something like that at hand?

scripts/cluster_network.py Outdated Show resolved Hide resolved
scripts/cluster_network.py Outdated Show resolved Hide resolved
scripts/cluster_network.py Outdated Show resolved Hide resolved
scripts/simplify_network.py Outdated Show resolved Hide resolved
@martacki
Copy link
Member Author

thanks @fneum for suggestions and improvements for the code, should all be in.

However, I want to make a change to the network "topology" sort-of and change two country-labels of n.buses. I'll do this in a new PR (somewhat cleaner), but the new PR should be in before this one.
HAC aggregates only connected nodes, but there are two nodes (one in HR, substation "Dakovo" and one in LU, substation "Vianden") that have no connections to other nodes within their own country. This throws a warning for HAC, because we do the clustering within each country seperately. It doesn't break HAC, but if HR/LU get assigned 2 nodes, this particlular one will never be aggregated and this is not the best partition.

I'll add some figs and definitely a note for the release_notes as well!

@martacki
Copy link
Member Author

martacki commented Feb 4, 2022

Ok, the following commit is rather ugly...

We can just remove the commit if we decide we don't want that "feature", but accounting for it results in a better representation of the grid topology of the network after clustering.

Here's a map extract for better reference.
LU+HR

@martacki
Copy link
Member Author

martacki commented Feb 4, 2022

and, as requested, here are some plots for comparison (k-meas vs. hac-time) for different spatial resolutions:

hac-time+kmeans_50.pdf
hac-time+kmeans_100.pdf
hac-time+kmeans_150.pdf

Copy link
Member

@fneum fneum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! But I still had a few comments and suggestions for improvements. Could you also add a release note with two or three sentences how to use it and what to keep in mind?

doc/configtables/clustering.csv Outdated Show resolved Hide resolved
doc/configtables/clustering.csv Outdated Show resolved Hide resolved
envs/environment.yaml Show resolved Hide resolved
envs/environment.yaml Outdated Show resolved Hide resolved
scripts/cluster_network.py Outdated Show resolved Hide resolved
scripts/cluster_network.py Outdated Show resolved Hide resolved
scripts/cluster_network.py Show resolved Hide resolved
scripts/simplify_network.py Outdated Show resolved Hide resolved
scripts/simplify_network.py Outdated Show resolved Hide resolved
scripts/cluster_network.py Outdated Show resolved Hide resolved
scripts/cluster_network.py Outdated Show resolved Hide resolved
scripts/cluster_network.py Outdated Show resolved Hide resolved
scripts/simplify_network.py Outdated Show resolved Hide resolved
@FabianHofmann FabianHofmann self-assigned this Mar 29, 2022
Copy link
Member

@fneum fneum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to merge this.

Although I've been thinking about whether it would be wise to first introduce this new feature and then make it a default in the version after the next. This would just mean changing the default config from hac to kmeans. What do you think @FabianHofmann @martacki?

@martacki could you also add a few lines and instructions to the release notes?

@martacki
Copy link
Member Author

Happy to merge this.

Although I've been thinking about whether it would be wise to first introduce this new feature and then make it a default in the version after the next. This would just mean changing the default config from hac to kmeans. What do you think @FabianHofmann @martacki?

@martacki could you also add a few lines and instructions to the release notes?

Yes I agree we shouldn't change default in the same go as we introduce the new clustering, just to be on the safer side. Changed to old default.

@fneum fneum merged commit f9a996e into master Jun 28, 2022
@fneum fneum deleted the introduce_hac_clustering branch June 28, 2022 05:44
fneum pushed a commit that referenced this pull request Mar 6, 2023
Integrate linopy as solver framework
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants