Closed brynpickering closed 11 months ago
Merging #466 (63df809) into main (cbed4c8) will decrease coverage by
2.79%
. Report is 9 commits behind head on main. The diff coverage is95.66%
.
@@ Coverage Diff @@
## main #466 +/- ##
==========================================
- Coverage 94.98% 92.20% -2.79%
==========================================
Files 31 32 +1
Lines 3890 4016 +126
Branches 0 957 +957
==========================================
+ Hits 3695 3703 +8
- Misses 195 198 +3
- Partials 0 115 +115
Files | Coverage Δ | |
---|---|---|
src/calliope/__init__.py | 100.00% <ø> (ø) |
|
src/calliope/_version.py | 100.00% <ø> (ø) |
|
src/calliope/backend/helper_functions.py | 99.35% <ø> (ø) |
|
src/calliope/backend/where_parser.py | 98.55% <100.00%> (ø) |
|
src/calliope/cli.py | 81.67% <100.00%> (ø) |
|
src/calliope/core/__init__.py | 100.00% <ø> (ø) |
|
src/calliope/core/attrdict.py | 99.50% <ø> (ø) |
|
src/calliope/core/io.py | 91.75% <100.00%> (ø) |
|
src/calliope/core/model.py | 94.05% <100.00%> (ø) |
|
src/calliope/core/util/generate_runs.py | 87.09% <ø> (ø) |
|
... and 22 more |
Did we now check to what extent this affects the actual mathematical model, and possible numerical issues?
Checked this by adding 26 new nodes to the national scale model (see overrides applied below) and then doing some memory profiling. It's not perfect because the new nodes don't actually add any complexity to the optimisation problem, but it gives a sense of memory footprint and whether CBC would trip up on the same min/max in the LP.
The result is less memory footprint when using min/max
not equals
!
memory footprints (MB, max / final allocations) using current main, where both equals
and min
/max
can be tested:
equals
: 166.9/166.9equals
: 159.9/159.9 <- 155.4 when using the implementation on this branch.equals
: 73.2/73.2equals
: 61.8/61.8Either way, CBC takes the same time to solve (~1 second) and the model takes the same time to load and build the problem.
So, seems a good idea to go ahead with this. I imagine that much bigger models might have different responses, but I can't test that.
@sjpfenninger this is now up-to-date and ready for review
Fixes issue #422
Summary of changes in this pull request:
_equals
variant removed in favour of using_min
and_max
simultaneously. For the most part this affects variable bounds, so will only increase model size if imposingenergy_cap_systemwide
orunits_systemwide
constraints, wherein two constraints are built instead of one. This is also not a massive issue since they are not built over the time dimensions so are unlikely to be particularly large constraint arraysenergy_cap_scale
andresource_scale
parameters removed as they introduce a maintenance burden when we do not know of any cases of their actual use.Reviewer checklist: