SpineOpt.jl
SpineOpt.jl copied to clipboard
WIP: Create new parameters for multi-year investments
Fixes #909
- [x] List the new parameters and relationships with explanations in this issue
- [x] Create the new parameters and relationships with their defaults in the SpineOpt template
- [x] Create a new case study in JSON format from the template with data (maybe future use as the tutorial)
- [x] Create a new file with the economic representation and add the function from #448
- [x] Refactor the code as suggested in #448
- [x] Create the new tests for the economic representation
- [ ] Create migration script for renamed parameters
Notice that with this issue, we just create the parameters and calculations but we don't use them yet in the model
Checklist before merging
- [ ] Documentation is up-to-date
- [ ] Unit tests have been added/updated accordingly
- [ ] Code has been formatted according to SpineOpt's style
- [ ] Unit tests pass
To update jsons to the latest template, use upgrade_all.jl
Codecov Report
Attention: Patch coverage is 87.16216%
with 38 lines
in your changes missing coverage. Please review.
Project coverage is 85.98%. Comparing base (
83ea1af
) to head (deaa53a
). Report is 159 commits behind head on master.
:exclamation: Current head deaa53a differs from pull request most recent head 87a2f99
Please upload reports for the commit 87a2f99 to get more accurate results.
Files | Patch % | Lines |
---|---|---|
src/data_structure/economic_structure.jl | 87.11% | 38 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #929 +/- ##
==========================================
- Coverage 86.71% 85.98% -0.73%
==========================================
Files 142 146 +4
Lines 3621 3576 -45
==========================================
- Hits 3140 3075 -65
- Misses 481 501 +20
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
After further discussion with @datejada, we concluded that:
- The ongoing implementation works well for only vintage investments, and eases the hassle for calculating the complicated economic parameters from the user.
The core of the current changes for multi-year investments is to correctly calculate the economic parameters (as done in this PR). In principle, one can also calculate these parameters a priori, i.e., using an excel sheet. In some cases when the temporal structure is constant, it might be more convenient to have a small function, rather than storing everything in (nested) maps. But for the moment this in turn would be kind of tricky when we move to time-varying structures...
- Vintage operation can be done from the database.
If we want to have vintage operation, we can define different units to represent vintage investments in the database, and then each unit would have a unique operational variable.
I did some benchmark using benchmark.jl
, which sets up a test database for 3 weeks (hourly resolution), 50 units, 50 nodes, and 49 connections. It is run 3 times and takes the average time. Here are the results:
Current test database | Modified test database | ||
---|---|---|---|
Master (not the latest) | This branch | Master (not the latest) | This branch |
187.267 s | 187.757 s | 180.680 s | 185.445 s |
To me, my branch does not seem to do much to performance. What do you think? @manuelma
I did some benchmark using
benchmark.jl
, which sets up a test database for 3 weeks (hourly resolution), 50 units, 50 nodes, and 49 connections. It is run 3 times and takes the average time. Here are the results:Current test database Modified test database Master (not the latest) This branch Master (not the latest) This branch 187.267 s 187.757 s 180.680 s 185.445 s To me, my branch does not seem to do much to performance. What do you think? @manuelma
Nice job @gnawin - why not comparing with master latest? where do the 5 seconds difference come from? Does the input system have a lot of investments or stuff that touches your new lines of code? Do you think 5 seconds is negligible?
I did some benchmark using
benchmark.jl
, which sets up a test database for 3 weeks (hourly resolution), 50 units, 50 nodes, and 49 connections. It is run 3 times and takes the average time. Here are the results: Current test database Modified test database Master (not the latest) This branch Master (not the latest) This branch 187.267 s 187.757 s 180.680 s 185.445 s To me, my branch does not seem to do much to performance. What do you think? @manuelmaNice job @gnawin - why not comparing with master latest? where do the 5 seconds difference come from? Does the input system have a lot of investments or stuff that touches your new lines of code? Do you think 5 seconds is negligible?
I can probably run a few more times to check further.
I did some more benchmark using benchmark.jl
, which sets up a test database for 3 weeks (hourly resolution), 50 units, 50 nodes, and 49 connections. It is run 10 times and takes the average time. Here are the results:
Modified test database | |
---|---|
Master (latest) | This branch |
23.472 s | 24.464 s |
I added this to the test database:
objs = [
["temporal_block", "two_year"],
]
rels = [
["model__default_investment_temporal_block", ["instance", "two_year"]],
]
These additions will trigger the creation of the parameters defined in economic_structure.jl
.
In summary, what I see is that indeed, creating new parameters bring extra time, but how significant this is and whether and how we should further improve the performance is not fully clear to me. Any ideas @manuelma?
One idea is that, we may continue with this work, and look back at performance when the variables and constraints are changed.
I did some more benchmark using
benchmark.jl
, which sets up a test database for 3 weeks (hourly resolution), 50 units, 50 nodes, and 49 connections. It is run 10 times and takes the average time. Here are the results:Modified test database Master (latest) This branch 23.472 s 24.464 s I added this to the test database:
objs = [ ["temporal_block", "two_year"], ] rels = [ ["model__default_investment_temporal_block", ["instance", "two_year"]], ]
These additions will trigger the creation of the parameters defined in
economic_structure.jl
.In summary, what I see is that indeed, creating new parameters bring extra time, but how significant this is and whether and how we should further improve the performance is not fully clear to me. Any ideas @manuelma?
One idea is that, we may continue with this work, and look back at performance when the variables and constraints are changed.
Good job @gnawin - I suggest you try and understand what is causing the slow down as soon as possible - it just gets more difficult as more things are added.
The benchmark you are using doesn't include investment decisions, does it? It is extremely important in my opinion to test the impact on a benchmark with investments, and on many different benchmarks. I think developing meaningful benchmarks should be a priority now, before this.
We are consistently improving performance in master and it would be a real pitty to merge something that sets us back.
Hi @manuelma, thanks for your comments. Actually, we agreed with @DillonJ that we need to increase the cases to benchmark to make it more useful. There is an issue with #1002; @gnawin and I can work on that, but it would be nice if you could be more specific about what "many" means for you since you are making most of the progress in the performance improvements in the code. What would you like to have benchmarked? So far, we have the following:
- Hourly temporal resolution, electricity only, 50 nodes, 50 units, 49 connections, and 3 weeks time horizon (but can increase it to any number...52? if we want a year)
We are thinking of adding:
- Investments for all the nodes, units, and connections
- Another case with a rolling horizon (a time block of one day and then moving forward one day until the end of the time horizon)
If you have more ideas, please comment on the #1002 issue and be as explicit as possible so we can implement it.
Hi @manuelma, thanks for your comments. Actually, we agreed with @DillonJ that we need to increase the cases to benchmark to make it more useful. There is an issue with #1002; @gnawin and I can work on that, but it would be nice if you could be more specific about what "many" means for you since you are making most of the progress in the performance improvements in the code. What would you like to have benchmarked? So far, we have the following:
- Hourly temporal resolution, electricity only, 50 nodes, 50 units, 49 connections, and 3 weeks time horizon (but can increase it to any number...52? if we want a year)
We are thinking of adding:
- Investments for all the nodes, units, and connections
- Another case with a rolling horizon (a time block of one day and then moving forward one day until the end of the time horizon)
If you have more ideas, please comment on the #1002 issue and be as explicit as possible so we can implement it.
Hi @datejada - those ideas seem good. I don't have anything specific in mind - what I mean by 'many' is we as a team try to collect the most relevant use cases. Maybe we can look at the Mopo case studies and find what's needed. I don't think I can come up with the list of comprehensive benchmarks myself.
Another thought is, the fact that I'm working on performance is probably a reason why I shouldn't be the one that comes up with the benchmarks. It is better in my opinion to define the benchmarks without knowledge of the implementation because that way it is easier to come up with something that actually challenges the implementation.
Hi @manuelma, thanks for your comments. Actually, we agreed with @DillonJ that we need to increase the cases to benchmark to make it more useful. There is an issue with #1002; @gnawin and I can work on that, but it would be nice if you could be more specific about what "many" means for you since you are making most of the progress in the performance improvements in the code. What would you like to have benchmarked? So far, we have the following:
- Hourly temporal resolution, electricity only, 50 nodes, 50 units, 49 connections, and 3 weeks time horizon (but can increase it to any number...52? if we want a year)
We are thinking of adding:
- Investments for all the nodes, units, and connections
- Another case with a rolling horizon (a time block of one day and then moving forward one day until the end of the time horizon)
If you have more ideas, please comment on the #1002 issue and be as explicit as possible so we can implement it.
Hi @datejada - those ideas seem good. I don't have anything specific in mind - what I mean by 'many' is we as a team try to collect the most relevant use cases. Maybe we can look at the Mopo case studies and find what's needed. I don't think I can come up with the list of comprehensive benchmarks myself.
Another thought is, the fact that I'm working on performance is probably a reason why I shouldn't be the one that comes up with the benchmarks. It is better in my opinion to define the benchmarks without knowledge of the implementation because that way it is easier to come up with something that actually challenges the implementation.
Hi @manuelma, sorry if it was not clear my comment. I agree that as a part of the MOPO project we want to make the benchmark more meaningful and we all should come up with ideas for that. I'm not asking you to define the benchmark cases, I'm asking for more ideas on top of what we have proposed. I think, that you can add value to what we want to achieve with the benchmarks, but if you think you shouldn't be involved, it is up to you.
So, moving forward, @gnawin and I will improve the benchmark test cases (#1002) to include the investment, and after that, we can benchmark this PR again and merge it (if there are no performance issues with it). Is that ok?
Hi @manuelma, thanks for your comments. Actually, we agreed with @DillonJ that we need to increase the cases to benchmark to make it more useful. There is an issue with #1002; @gnawin and I can work on that, but it would be nice if you could be more specific about what "many" means for you since you are making most of the progress in the performance improvements in the code. What would you like to have benchmarked? So far, we have the following:
- Hourly temporal resolution, electricity only, 50 nodes, 50 units, 49 connections, and 3 weeks time horizon (but can increase it to any number...52? if we want a year)
We are thinking of adding:
- Investments for all the nodes, units, and connections
- Another case with a rolling horizon (a time block of one day and then moving forward one day until the end of the time horizon)
If you have more ideas, please comment on the #1002 issue and be as explicit as possible so we can implement it.
Hi @datejada - those ideas seem good. I don't have anything specific in mind - what I mean by 'many' is we as a team try to collect the most relevant use cases. Maybe we can look at the Mopo case studies and find what's needed. I don't think I can come up with the list of comprehensive benchmarks myself. Another thought is, the fact that I'm working on performance is probably a reason why I shouldn't be the one that comes up with the benchmarks. It is better in my opinion to define the benchmarks without knowledge of the implementation because that way it is easier to come up with something that actually challenges the implementation.
Hi @manuelma, sorry if it was not clear my comment. I agree that as a part of the MOPO project we want to make the benchmark more meaningful and we all should come up with ideas for that. I'm not asking you to define the benchmark cases, I'm asking for more ideas on top of what we have proposed. I think, that you can add value to what we want to achieve with the benchmarks, but if you think you shouldn't be involved, it is up to you.
So, moving forward, @gnawin and I will improve the benchmark test cases (#1002) to include the investment, and after that, we can benchmark this PR again and merge it (if there are no performance issues with it). Is that ok?
Sounds good, but don't forget the migration. Also, the style.
CI tests are not passing now, it seems that it is a compatibility issue with the version of SpineToolbox. Because locally I have SpineToolbox 0.7.4, and all tests are passing.
Some further benchmarking using the following set-up:
url_in_invest, url_out_invest = setup(number_of_weeks=1, n_count=5, add_investment=true, add_rolling=false)
SUITE["main", "run_spineopt", "investment"] =
@benchmarkable run_spineopt($url_in_invest, $url_out_invest; log_level=3, optimize=false) samples = 3 evals = 1 seconds =
Inf
Runs | Test case that includes investment decisions | |
---|---|---|
Master | This branch | |
1st | 16.528 s | 16.812 s |
2nd | 16.924 s | 15.851 s |
Probably will do more benchmarks using larger cases.
Remaining tasks:
- [x] Update the examples to the new format with the new parameters
- [x] Run the benchmark in a large test case
- [x] Performance checks (especially stochastic scenarios/structures)
- [x] Fix tests with the latest updates for 0.8.2
- [x] Review code style
- [x] Migration script with latest updates for 0.8.2
Update on the benchmarking using the following set-up:
# basic run
url_in_basic, url_out_basic =
setup(number_of_weeks=8, n_count=100, add_meshed_network=true, add_investment=false, add_rolling=false)
# with investment
url_in_invest, url_out_invest = setup(number_of_weeks=4, n_count=10, add_investment=true, add_rolling=false)
Run | Master | This branch (no economic parameters) | This branch (with economic parameters) |
---|---|---|---|
Basic | 927.731 s | 924.621 s | 895.806 s |
Investment | 2029.441 s | 2063.121 s | 2038.750 s |
Generally, the runs show no major impact when adding the economic parameters. There is a variability that we can reduce by increasing the number of samples in the benchmark. But, in general terms, we can see no hampering on the master branch performance.
@manuelma, according to the previous comments, we finally have this PR finished. The branch has been benchmarked with and without calculating the new parameters. There is no significant impact on the master branch (see the results in a couple of comments above). @gnawin has formatted the code in the SpineOpt style, and by default, SpineOpt does not calculate the economic parameters, so there is also no impact on existing SpineOpt projects. A migration script also updates the name of AAA_investment_lifetime
to AAA_investment_tech_lifetime
(AAA being unit, connection, or storage).
So, do you have any last comments? otherwise, we will continue with the merge of this PR and the work on modelling the multi-year investment in SpineOpt, see #908