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

Add commodity flow of capacity variables #451

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

Add commodity flow of capacity variables #451

wants to merge 32 commits into from

Conversation

Jihoon
Copy link
Contributor

@Jihoon Jihoon commented Mar 31, 2021

This PR cleans up and supersedes #387.

PR checklist

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

Things to review

  • The formulation and the documentation is explanatory about the new changes that are made.
  • Check if there are any issues that can possibly arise from the new formulation. (any cases where it might not work as expected).
  • Building and solving a scenario with the new formulation without any erorrs (can be the Westeros tutorial).

Expected review time: One hour or one and a half hour

@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #451 (6fe208a) into main (6fe208a) will not change coverage.
The diff coverage is n/a.

❗ Current head 6fe208a differs from pull request most recent head daafbd4. Consider uploading reports for the commit daafbd4 to get more accurate results

@@          Coverage Diff          @@
##            main    #451   +/-   ##
=====================================
  Coverage   93.0%   93.0%           
=====================================
  Files         43      43           
  Lines       3373    3373           
=====================================
  Hits        3137    3137           
  Misses       236     236           

@khaeru
Copy link
Member

khaeru commented Apr 2, 2021

#452 is modifying the CI setup. Please restart the tests here (manually or via rebase) once that one is merged, hopefully within an hour.

Copy link
Member

@khaeru khaeru left a comment

Choose a reason for hiding this comment

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

I edited the PR description to re-add the checklist that's auto inserted by the template. These items, particularly documentation, are essential—even for minor PRs. This one is not minor, because it alters the core model formulation.

At the absolute minimum, when completing the item “Update release notes,” mention the parameters, variables, and equations that are added/affected by the additions here. Then, users might have a thin hope of understanding that changes have been made, and might try to puzzle them out or even use them.

Much better would be to update the inline documentation in the GAMS files, or add a new file in docs/, with plain-language description. For a good example, see #190 by @behnam-zakeri: https://github.com/iiasa/message_ix/pull/190/files, in particular

  • additions in model_core.gms and parameter_def.gms.
  • how the corresponding item in the release notes links to these descriptions.

As a reminder, if time feels tight, there is always the option to task a specific colleague with some or all of this work.

@volker-krey volker-krey added this to the 3.3 milestone May 5, 2021
@khaeru
Copy link
Member

khaeru commented May 24, 2021

Postponed to v3.4 because:

  • PR checklist is incomplete.
  • 3 requested reviews not done.
  • Requested changes from 1 review not completed.

@khaeru khaeru modified the milestones: 3.3, 3.4 May 24, 2021
@LauWien
Copy link
Contributor

LauWien commented Jan 19, 2022

I've added the WIP tag, since there is still more time/investigation needed to finish up the PR, as discussed in today's MESSAGEix meeting. Due to that and the following

  • PR checklist is incomplete,
  • 3 requested reviews not done,
  • Requested changes from 1 review not completed,

we postpone to v3.5.

@LauWien LauWien modified the milestones: 3.4, 3.5 Jan 19, 2022
* CAP_NEW(location,tec,year) ) \
* commodity input and output associated with retirement of technology capacity (via differentials of capacity of successive periods)
* for first model period (differential with historical remaining capacity)
+ SUM( (location,tec,vintage,year2)$( inv_tec(tec) AND map_tec_lifetime(location,tec,vintage,year2) \
Copy link
Contributor

@GamzeUnlu95 GamzeUnlu95 Feb 11, 2022

Choose a reason for hiding this comment

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

map_tec_lifetime issue for the first year scrap release equation:
If we say the first model year is 700 and the vintage is 690:
According to this equation which is valid only for the first model year, based on the conditions specified here
year = 700, year2 = 690. In this case for a technology that has vintage 690 map_tec_lifetime(location,tec,690,690) becomes a condition that needs to be satisfied. And map_tec_lifetime does not have this entry for vintage years. In this case there will be no scrap released for the first model year.

As another example, if we say the vintage year is 680 and the lifetime is 20 years, the only way to make sure this plant will retire in the first model period (700) is to check map_tec_lifetime and see the entry for (680, 690). Otherwise, we don't know when it is supposed to retire. We either need to make this historical vintage year addition to map_tec_lifetime or change the equations to satisfy the necessary condition.

Copy link
Contributor Author

@Jihoon Jihoon Feb 14, 2022

Choose a reason for hiding this comment

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

Just to add a bit on Gamze's comment above, for the capacity supposed to end its activity in 690 and retire in 700, the historical capacity that existed in 690 cannot be taken into account, because the map_tec_lifetime condition does not pass for the period pair of (year2, year) = (690, 700). map_tec_lifetime(location,tec,vintage,year2) is only defined for year2 >= fmy = 700.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even if the necessary map_tec_lifetime is added manually in GAMS, the if conditions are not satisfied and no scrap is released in the first year. Not sure why this is happening... The equations printed in the .lst format are attached. The gdx files are in: I:\unlu\power_sector_integration\24.05

MESSAGE_run_test_2405.txt

AND first_period(year) AND seq_period(year2,year) ), \
* output by all new capacity of technologies located at 'location' sending to 'node' and 'time' distributed over years of periods
output_cap_ret(location,tec,vintage,node,commodity,level,time) \
* ( remaining_capacity(node,tec,vintage,year2) - CAP(location,tec,vintage,year) ) \
Copy link
Contributor

@GamzeUnlu95 GamzeUnlu95 Feb 11, 2022

Choose a reason for hiding this comment

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

Reformulation of the first year scrap release equation:
The remaining capacity seems to be a binary value, does it make sense to perform a subtraction such as
( remaining_capacity(node,tec,vintage,year2) - CAP(location,tec,vintage,year) )?

Copy link
Contributor

@GamzeUnlu95 GamzeUnlu95 May 24, 2022

Choose a reason for hiding this comment

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

Here we can use something like:

  • output by all new capacity of technologies located at 'location' sending to 'node' and 'time' distributed over years of periods
        output_cap_ret(location,tec,vintage,node,commodity,level,time)                                                 
        ( historical_new_capacity(node,tec,vintage) - CAP(location,tec,vintage,year)/ duration_period(year) )  
  • input by all new capacity of technologies located at 'location' taking from 'node' and 'time' distributed over years of periods
        - input_cap_ret(location,tec,vintage,node,commodity,level,time)                                               
        (historical_new_capacity(node,tec,vintage) - CAP(location,tec,vintage,year)/ duration_period(year))) 

We might need to sum the historical_new_capacity over vintage years (if there are two different vintages with different life times both retiring in the first model year...)

/ duration_period(year) ) \
* for other model periods (differential with installed capacity of preceding period)
+ SUM( (location,tec,vintage,year2)$( inv_tec(tec) AND map_tec_lifetime(location,tec,vintage,year2) \
AND NOT first_period(year) AND seq_period(year2,year) ), \
Copy link
Contributor Author

@Jihoon Jihoon Feb 14, 2022

Choose a reason for hiding this comment

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

Another suspected issue here is:
For the capacity (e.g. vintage=740) ending its activity in e.g. year2=750, because the condition seq_period(750,760) still passes, this part of the equation keeps the term -CAP(location,tec,740,760) (in line 613), which we expect to be 0 by definition. But differently from our expectation, it appears that while solving the LP model, a spurious non-zero value is assigned to this CAP(location,tec,740,760).

@volker-krey
Copy link
Contributor

The issue related to the historical vintage 690 of coal_ppl seems to be a result of the missing technical_lifetime parameter of that vintage in the Westeros tutorial (see #609).

The problem of non-zero CAP values of technologies after the end of their lifetime is due to the fact that there is no equation that would require CAP to be zero after the end of the technology lifetime. The equation CAPACITY_MAINTENANCE just requires that CAP in a period is lower than in a previous period. This usually does not lead to problems, but in this extension there might be a value in delaying commodity release to better balance a "scrap-type" commodity equation that is set to equal. Usually, there is no cost penalty and also no other benefits as other parameters have not been defined for periods beyond the lifetime.

To avoid this behavior, we may need to introduce an additional equation of the following kind:

CAPACITY_MAINTENANCE2(node,inv_tec,vintage,year)$( map_tec(node,inv_tec,vintage) AND NOT map_tec_lifetime(node,inv_tec,vintage,year) 
AND NOT first_period(year) AND year_order(vintage) < year_order(year))..
    CAP(node,inv_tec,vintage,year) =L= 0 ;

volker-krey and others added 29 commits July 17, 2024 13:26
- scaling factor adjusts for difference in length of construction and retirement periods of capacities in related commodity flows
- added missing closing parenthesis to sums in commodity flow
- scaling factor adjusts for difference in length of construction and retirement periods of capacities in related commodity flows
- added missing closing parenthesis to sums in commodity flow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enh New features & functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants