message_ix icon indicating copy to clipboard operation
message_ix copied to clipboard

Correct levelized_cost calculation

Open OFR-IIASA opened this issue 3 years ago • 5 comments

As explained in issue #652, this PR aims to adjust the calculation of levelized_costs, so that these are not only calculated when investment costs are parametrized for a technology, but also when variable costs only have been defined.

How to review

  • Read the diff and note that the CI checks all pass. A Specific test cannot be written because it is not possible to retrieve the parameter "levelized_cost" from the scenario object.

PR checklist

  • [x] Continuous integration checks all ✅ ~- [ ] Add or expand tests; coverage checks both ✅~
  • [x] Add, expand, or update documentation.
  • [x] Update release notes.

OFR-IIASA avatar Sep 21 '22 14:09 OFR-IIASA

Codecov Report

Merging #653 (96b1c86) into main (27cd567) will decrease coverage by 0.0%. The diff coverage is n/a.

:exclamation: Current head 96b1c86 differs from pull request most recent head a1f576b. Consider uploading reports for the commit a1f576b to get more accurate results

@@           Coverage Diff           @@
##            main    #653     +/-   ##
=======================================
- Coverage   93.0%   93.0%   -0.1%     
=======================================
  Files         43      43             
  Lines       3419    3378     -41     
=======================================
- Hits        3183    3142     -41     
  Misses       236     236             
Impacted Files Coverage Δ
message_ix/message_ix/tests/test_nightly.py 50.0% <0.0%> (-4.6%) :arrow_down:
message_ix/message_ix/testing/nightly.py 45.9% <0.0%> (-3.0%) :arrow_down:
message_ix/message_ix/tools/add_year/cli.py 87.3% <0.0%> (-0.2%) :arrow_down:
message_ix/message_ix/cli.py 92.5% <0.0%> (-0.2%) :arrow_down:
message_ix/message_ix/macro.py 96.3% <0.0%> (-0.2%) :arrow_down:
message_ix/message_ix/core.py 95.0% <0.0%> (-0.1%) :arrow_down:
message_ix/message_ix/tests/tools/test_add_year.py 95.7% <0.0%> (-0.1%) :arrow_down:
message_ix/message_ix/tests/test_macro.py 99.4% <0.0%> (-0.1%) :arrow_down:
message_ix/message_ix/models.py 100.0% <0.0%> (ø)
message_ix/message_ix/tests/conftest.py 100.0% <0.0%> (ø)
... and 3 more

codecov[bot] avatar Sep 21 '22 14:09 codecov[bot]

@behnam-zakeri as discussed yesterday, let us extend the calculation of the levelized costs to include the relation-related costs as well. Also, the issues raised on @khaeru can also be addressed in this PR:

  • [x] This PR resulted in a line (the one for https://github.com/iiasa/message_ix/pull/645) appearing twice in the release notes.
  • [x] Some of the edits to doc/macro.rst result in a sentence appearing in all bold here.

OFR-IIASA avatar Oct 06 '22 05:10 OFR-IIASA

This seems like a key update that should be tested if at all possible. I wonder if it's possible to use the existing dantzig problem to test levelized cost values with

  • inv_cost only
  • fix_cost only
  • both inv_cost and fix_cost

gidden avatar Oct 08 '22 08:10 gidden

@gidden I agree, and also to test the 'var_cost' as well as the additions to the formulation related to relations. I believe by solving using the command solve(var_list=["levelized_cost"]) we will hopefully be able to retrieve the levelized_cost from the scenario object.

OFR-IIASA avatar Oct 09 '22 07:10 OFR-IIASA

In order to address the issues associated with creating tests for this PR, an issue has been started, see #661 Once the issues have been resolved and the parameter can be retrieved, tests should be written to verify the levelized_cost.

OFR-IIASA avatar Oct 18 '22 07:10 OFR-IIASA

@behnam-zakeri , my proposal would be to merge this PR first and then change the PR for #658 to main. The reason is that while here we are only just changing "when" the levelized_cost are being calculated, the changes in #658 change how these are calculated. I would therefore propose to add tests for the other PR, but for some changes on the java end are required to allow doing this. I therefore propose to merge this PR as is and continue work on #658 thereafter.

OFR-IIASA avatar Oct 31 '22 10:10 OFR-IIASA

The failed test here is from a test timeout, e.g.,

FAILED message_ix/tests/test_tutorials.py::test_tutorial[westeros_addon_technologies] - nbclient.exceptions.CellTimeoutError: A cell timed out while it was being executed, after 10 seconds.

gidden avatar Nov 07 '22 16:11 gidden

Noting previous tests passed except the noted failures, merging.

gidden avatar Nov 07 '22 17:11 gidden