Make extendability and committability compatible, when modularity is on#1007
Make extendability and committability compatible, when modularity is on#1007FabianHofmann merged 89 commits intoPyPSA:masterfrom
Conversation
davide-f
left a comment
davide-f
left a comment
There was a problem hiding this comment.
Thanks @enricogiglio :D
I've gone through the PR and added some preliminary comments.
Please, see that the pre-commit has added edits.
Do:
git fetch
git pull
to have them locally.
Afterwards, ensure to install the pre-commit locally using:
pre-commit install
This should prevent future online commits related to the precommit
…es_but_non_modular
Co-authored-by: Giovanni Zaccaria <[email protected]>
|
@davide-f I integrated the changes you suggested as discussed, but 1) we also added more tests and 2) now we have passed all the checks. Any other suggestions? |
To me the PR looks ready to be revised by the maintainers. |
- Add fixtures for common network setups - Add helper functions for generator/link creation - Reduce snapshot counts for faster execution - Consolidate similar tests via parametrization - Reduce test code by 62% (1731 → 662 lines)
lkstrp
left a comment
lkstrp
left a comment
There was a problem hiding this comment.
This will make a couple of people happy if finally merged ! There are some formatting things in the docs left, which are otherwise lovely. And then I have some pain with all the validation/ consistency stuff, but I think this needs a bigger clean up in a follow up PR and not in here. Just waiting for the formulation review of @Irieo
There was a problem hiding this comment.
This is a really neat addition (and generalisation) of PyPSA UC & investment logic functionality, h/t @enricogiglio and @FabianHofmann. Implementation makes good sense, the notebooks illustrate new functionality neatly. The inference of M for Big-M is 🍒 (one can argue if the implementation is enough tight, but the very fact it is automatically found and not hardcoded is great).
Below is the review, here is brief summary:
- I suppose some math in docs is wrong, but code implementation is actually correct.
- note that ramp_limit_start_up/down are not working here (reason/fix is in #1540). More, compatibility with ramp_limit_start_up/down is not mentioned in summary table in docs and not tested (I left dedicated comment for it)
- The fact that in modular expansion case the status variable is NOT a binary on/off variable anymore, but instead becomes an integer representing the number of committed modules inside the component & the fact that status defaults to "1" -> creates a super unintuitive result, also observed in the notebook here. Say we invest into 5 blocks x 100 MW to cover 500 MWh/h load. The reported status and dispatch variables are 5 & 500 MW, but there are 4 startups (since first one is already started up), which also means we pay in objective for 4 UC events and the first one PyPSA gives for free 🎁. User can control it by adjusting default status obv, and I don't think this is something worth fixing, but it is worth documenting.
- There are no tests for multi-period invesment logic and considering the madness that can be created by transition of all status variables across periods across modular units correctly, I'd recommend covering this with tests.
add redirect from PyPSA/PyPSA#1007
|
@Irieo Adding a comment on
That phenomenon is intended, though agree worth documenting. With modularity options, a single PyPSA object simulates the dispatch of N independent units, which saves lots of variables (continuous and binary ones). As such, if any of the unit is dispatched, then the startup cost is paid. |
Avoid LinearExpression * LinearExpression in ramp constraints when both extendable and committable components coexist. For modular+ committable+extendable use p_nom_mod as constant capacity per module.
Document all four big-M ramp constraints (run/start-up/shut-down) with correct constraint names and startup/shutdown ramp limit parameters. Add test for modular+committable ramp limits.
|
🚀 |
7 participants
Closes #1000
@davide-f and I would like to propose a MILP formulation for extendability and committability, only when modularity is on, to make these two features compatible. We have also added a test for the feature we have implemented, and believe that the proposed changes are consistent with the rest of the codebase. In particular, the goal is to keep everything compatible with the previous version without introducing breaking changes.
At the moment we'd like to continue working on improving the proposed code changes, but perhaps we'll also propose a new test that's more accurate with respect to the changes introduced.
Checklist
doc.environment.yaml,environment_docs.yamlandsetup.py(if applicable).doc/release_notes.rstof the upcoming release is included.