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

[benchmarks/pypsa] Simplify config files and shrink benchmarks #35

Merged
merged 22 commits into from
Nov 1, 2024

Conversation

danielelerede-oet
Copy link
Contributor

Hi @siddharth-krishna , in the last commit I removed those lines that pypsa-eur-elec-10-lvopt-3h.yaml has in common with the common.default.yaml file for pypsa v 0.11.0 (which is the one this config file refers too). However, apart from reducing the size of the file I'm not so convinced of the reason why we should need to cut parts of this file. I guess we'll be having a reference config.default.yaml file and an additional script to modify it according to the specific info related to each of the pypsa-eur-based sample problems, but I can't see why we should be doing that, if so.

Copy link
Contributor

@siddharth-krishna siddharth-krishna left a comment

Choose a reason for hiding this comment

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

@danielelerede-oet this looks good to me. I think the motivation for removing the common lines with the default config is so that it is easier for humans to look at the different benchmark configs and visually see what each of them is about and how they differ.

I don't think we need to have an additional script that modifies this config and brings in info from the default config -- I think snakemake does that automatically. Could you please check that you are able to build the same model using the reduced config compared to the config on the main branch?

@siddharth-krishna siddharth-krishna changed the title Daniele/pypsa problems [benchmarks/pypsa] Simplify config files Sep 25, 2024
@danielelerede-oet
Copy link
Contributor Author

Hi @siddharth-krishna, the modifications in the latest commit comply with the following requirements:

  1. modify the metadata as in contrast with Fabian's first classification the problem with transmission expansion is actually not a MILP
  2. update the pypsa-eur-based UC problem removing the linearization of UC
  3. change the rules for solving the operational problems given the latest updates to pypsa-eur

Moreover, I'm currently experiencing issues with the solve_operations_network rule in pypsa-eur after the latest changes done in the repo. I already opened an issue about that (PyPSA/pypsa-eur#1344).

Copy link
Contributor

@siddharth-krishna siddharth-krishna left a comment

Choose a reason for hiding this comment

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

Thanks, Daniele! I notice a potential bug with the linearized_unit_commitment line, otherwise it looks good.

I assume the plan is to wait for the pypsa-eur issue to be fixed before we merge in this PR?

benchmarks/pypsa/pypsa-eur-elec-20-lv1-3h-op-ucconv.yaml Outdated Show resolved Hide resolved
benchmarks/pypsa/pypsa-eur-elec-20-lv1-3h-op-ucconv.yaml Outdated Show resolved Hide resolved
@danielelerede-oet
Copy link
Contributor Author

@siddharth-krishna today I updated metadata with continuous and binary variables for MILP problems (I also noticed there was a mistake before because I didn't know how to properly read the number of variables, but Julian gave wonderful suggestions on this) and resolved a conflict in the solution of pypsa-gas+wind+sol+ely-1h-ucgas.py (that doesn't change anything at all in the formulation of the problem, it's just that pypsa was raising warnings due to this). I fear anyway that the "only_generate_problem_file" option doesn't work for pypsa-gas+wind+sol+ely-1h-ucgas.py and pypsa-gas+wind+sol+ely-1h.py

@danielelerede-oet
Copy link
Contributor Author

danielelerede-oet commented Oct 17, 2024

Hi @siddharth-krishna, I changed the names for pypsa benchmarks according to our discussion and updated metadata. I guess the runner and maybe some other files require some modifications, too related to this. I also removed information about the absence of transmission expansion (formerly indicated through lv1) and, when present, I'm now identifying it through trex (let me know if you have better ideas) instead of lvopt which is very "pypsa-sounding".

@siddharth-krishna
Copy link
Contributor

I'm merging this PR now in because we need to build on it for upcoming PRs. Daniele, when you're back next week, could you perhaps review my changes and let me know if you'd like any changes? I can do them in another PR.

My main change was to reduce nodes and increase time resolution to make all the benchmarks as small as possible. This is in order to make all solvers solve them in a short amount of time, which eases our testing. One of my next work items is to create multiple sizes of each benchmark, so the original sizes will be re-added to the benchmark set soon. I hope that's okay.

I notice there's a bug with the GLPK results: all the statuses are warning and not ok. I'll open an issue to investigate this.

@siddharth-krishna siddharth-krishna changed the title [benchmarks/pypsa] Simplify config files [benchmarks/pypsa] Simplify config files and shrink benchmarks Nov 1, 2024
@siddharth-krishna siddharth-krishna merged commit 19fce1f into main Nov 1, 2024
1 check passed
@siddharth-krishna siddharth-krishna deleted the daniele/pypsa-problems branch November 1, 2024 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants