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

Missing arguments in simplify_network's cluster and clustering_for_n_clusters #1268

Open
2 tasks done
LRydin opened this issue Dec 27, 2024 · 3 comments
Open
2 tasks done
Labels
bug Something isn't working

Comments

@LRydin
Copy link

LRydin commented Dec 27, 2024

Checklist

  • I am using v0.5.2 and to the best of my knowledge, this issue is valid for v0.6.0.
  • I am running on a clean pypsa-earth environment, on a linux machine conda env -f envs/linux-pinned.yaml.

Describe the Bug

TL;DR:

  • args inputs and config missing on Lines 609-624 and Lines 651-667 in simplify_network.py
  • inputs, outputs, config = snakemake.input, snakemake.output, snakemake.config missing on Line 970 in simplify_network.py
  • loose kwarg potential_mode in simplify_network.py not in cluster_network.py (to the best of my knowledge).

Potential incongruence between clustering_for_n_clusters in simplify_network.py and cluster_network.py.

see Line 557 for clustering_for_n_clusters in cluster_network.py, where we read:

def clustering_for_n_clusters(
    inputs,
    config,
    n,
    n_clusters,
    alternative_clustering,
    gadm_layer_id,
    geo_crs,
    country_list,
    distribution_cluster,
    build_shape_options,
    custom_busmap=False,
    aggregate_carriers=None,
    line_length_factor=1.25,
    aggregation_strategies=dict(),
    solver_name="cbc",
    algorithm="kmeans",
    feature=None,
    extended_link_costs=0,
    focus_weights=None,
):

Now note the call in Line 651 to clustering_for_n_clusters in simplify_network.py, which seems to miss two args (inputs and config) and includes a kwarg potential_mode that I cannot trace:

clustering = clustering_for_n_clusters(
    n,
    n_clusters,
    alternative_clustering,
    gadm_layer_id,
    geo_crs,
    country_list,
    distribution_cluster,
    build_shape_options,
    custom_busmap=False,
    aggregation_strategies=aggregation_strategies,
-    potential_mode=potential_mode,
    solver_name=solver_name,
    algorithm=algorithm,
    feature=feature,
    focus_weights=focus_weights,
)

Potential solution

I believe that:

inputs, outputs, config = snakemake.input, snakemake.output, snakemake.config

is needed in __main__ of simplify_network.py, e.g. around Line 970. Furthermore, the two args inputs and config should be inserted in Line 609-624

def cluster(
+    inputs,
+    config,
    n,
    n_clusters,
    alternative_clustering,
    build_shape_options,
    country_list,
    distribution_cluster,
    focus_weights,
    gadm_layer_id,
    geo_crs,
    renewable_config,
    solver_name,
    algorithm="hac",
    feature=None,
    aggregation_strategies=dict(),
):

and similarly in Line 651-667, as

    clustering = clustering_for_n_clusters(
+      inputs,
+      config,
        n,
        n_clusters,
        alternative_clustering,
        gadm_layer_id,
        geo_crs,
        country_list,
        distribution_cluster,
        build_shape_options,
        custom_busmap=False,
        aggregation_strategies=aggregation_strategies,
-        potential_mode=potential_mode,
        solver_name=solver_name,
        algorithm=algorithm,
        feature=feature,
        focus_weights=focus_weights,
    )
@LRydin LRydin added the bug Something isn't working label Dec 27, 2024
@ekatef
Copy link
Member

ekatef commented Dec 27, 2024

Hello @LRydin, and thanks a lot for notifying!

I'm right now working on the task which is closely linked with the issue you have traced. Not yet sure exactly what is the matter, and happy to keep you updated.

@LRydin
Copy link
Author

LRydin commented Dec 28, 2024

Hey @ekatef, that's wonderful. I think I will also be trying a few more simplify network problems. I have also encountered some elementary issues with the clustering to GAMD with the alternative_clustering option. If it helps, I can details my findings here.

@ekatef
Copy link
Member

ekatef commented Dec 28, 2024

Hey @ekatef, that's wonderful. I think I will also be trying a few more simplify network problems. I have also encountered some elementary issues with the clustering to GAMD with the alternative_clustering option. If it helps, I can details my findings here.

It would be very much appreciated if you would be interested investigate how alternative clustering work. At the moment, we are aware that there are some issues for sure. A discussion here gives some context and lists a possible strategy to improve. Personally, I also expect that some refactoring will be also beneficial (as you have just demonstrated 🙂).

Looking forward for the developments and happy to discuss the results!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants