Develop-sticky merge #199

emolinab · 2020-07-02T12:24:50Z

Description

The following PR includes the incorporation of the new "sticky" realization for calculation of factor costs, and the way modules with investment decisions account for overall costs.

Purpose of this PR

The purpose is to improve MAgPIE's performance in the spatial sense, favoring crop area expansion in cells with preexisting farmland and capital. Also, to improve the calculation of costs where investment decisions are taken.

Performance loss/gain from the current default behavior

Default run time: 0.7 hours
New PR: 1.4 hours
Comparison: 1.8 times slower.

Type of change

Bug fix (Change which fixes an issue).
New feature (Change which adds functionality).
Minor change (Change which does not modify core model code i.e., in /modules).
Major change (fix or feature that would change current model behavior).

How Has This Been Tested?

Runs using starting script which tests current defaults (using test_runs.R).
Runs using starting script which successfully runs the same scenarios as in test_runs.R but with the changes from PR.

Default configuration additions/changes:

Realizations:
Scalars/Constants:
Other changes:

Checklist:

Self-review of my own code.
Added changes to CHANGELOG.md
No hardcoded numbers and cluster/country/region names.
The code doesn't contain declared but unused parameters or variables.
In-code comments added including documentation comments.
Compiled and checked resulting documentation with goxygen::goxygen() for the new/updated parts.
Changes to magpie4 R library for post-processing of model output (ideally backward compatible).

Special comments/warnings

The use of sticky increases run times, and overall cost due to larger input costs. Please, be cautious when you use region setting different to the usual ones.

…p' into develop

…r/develop' into develop_sticky_merge

…cky_merge

tscheypidi · 2020-07-02T12:44:20Z

Just for clarification: You ticked all categories for Default configuration additions/changes which would mean that you changed default settings in all categories, but my understanding is that you did not touch any default settings. Am I right? If so, please correct your description of the pull request and untick the corresponding categories.

In addition, please do not delete lines from the template which contain check boxes. Just keep the lines unchecked to make clear what does not apply for your PR.

emolinab · 2020-07-02T13:16:55Z

Thanks Jan! Issues fixed. You are right, I didn't change any of the default configurations, so those items are now unticked, and I have left the unchecked items in the "Type of change" category.

tscheypidi

looks good, only some minor requests from my side before it can be merged

tscheypidi · 2020-07-02T13:49:25Z

CHANGELOG.md

+
+**scripts** removed GP_final start script due to obsolescence
+
+


add your changes to the existing unreleased section instead of introducing a second unreleased section

tscheypidi · 2020-07-02T13:51:01Z

main.gms

@@ -197,7 +197,7 @@ $title magpie
 * 
 * 
 * 
-* Last modification (input data): Thu Jun 18 18:28:38 2020
+* Last modification (input data): Thu Jun 25 17:37:27 2020
 * 


ideally, you should not add files to your commit with irrelevant changes such as the modification date update. In case of main.gms it should be in most cases excluded from a commit.

You can keep it in the PR this time, but please avoid adding such files next time.

Perfect, I'll keep it in mind for the next one.

tscheypidi · 2020-07-02T13:53:54Z

modules/38_factor_costs/fixed_per_ton_mar18/declarations.gms

+;
+
+parameters
+ p38_ovcosts(t,i,kcr)          Overall factor costs (mio USD05MER)


is p38_ovcosts required/used in the fixed case?

tscheypidi · 2020-07-02T13:54:23Z

modules/38_factor_costs/fixed_per_ton_mar18/postsolve.gms

@@ -5,15 +5,19 @@
 *** |  MAgPIE License Exception, version 1.0 (see LICENSE file).
 *** |  Contact: magpie@pik-potsdam.de

+p38_ovcosts(t,i,kcr)   = vm_cost_prod.l(i,kcr);


what is this used for?

p38_ovcost originally accounts for the overall costs coming from sticky. Since I modified the costs.R function in magpie4 to read p38_ovcosts, I used it in the postsolve of all the factor costs realizations.

for reporting please use ov_cost_prod instead (and remove p38_octcosts again)...it contains the values of vm_cost_prod for all time steps.

you mean for the fixed and mixed realizations of factor costs, or for all the modules and realizations where I used these pxx_overall_costs parameters?

If the costs are not influencing the subsequent periods, then it should not be in the gams code.
all "pure" prosprocessing can happen in R, based on the level values of the variables.

from what I saw, it is only used in postprocessing. Then remove this parameter from the GAMS code and make the calculations in the magpie4 library.

tscheypidi

looks good to me

modules/38_factor_costs/sticky_feb18/declarations.gms

modules/38_factor_costs/sticky_feb18/equations.gms

bodirsky

in a run with different region setting, the calibration runs become infeasible.
Feli and I tracked it down:

its the upper bound for the investment constraint in the presolve. We have small region in africa (ruanda, uganda, tanzania) where this constraint is too narrow, as investments are higher than the defined gdp share.

Not sure whether this is only in the calibration, or in general with the model. The constraint should be relaxed.

bodirsky · 2020-07-03T13:22:49Z

modules/38_factor_costs/fixed_per_ton_mar18/postsolve.gms

@@ -5,15 +5,19 @@
 *** |  MAgPIE License Exception, version 1.0 (see LICENSE file).
 *** |  Contact: magpie@pik-potsdam.de

+p38_ovcosts(t,i,kcr)   = vm_cost_prod.l(i,kcr);


from what I saw, it is only used in postprocessing. Then remove this parameter from the GAMS code and make the calculations in the magpie4 library.

…cky_merge

emolinab · 2020-07-22T10:39:29Z

in a run with different region setting, the calibration runs become infeasible.
Feli and I tracked it down:

its the upper bound for the investment constraint in the presolve. We have small region in africa (ruanda, uganda, tanzania) where this constraint is too narrow, as investments are higher than the defined gdp share.

Not sure whether this is only in the calibration, or in general with the model. The constraint should be relaxed.

I have looked into it, and the maximum value of the capital investment in Feli's runs is close to 11%. I had set up this value for 5% for the default region setting (based on previous runs). I am thinking about increasing this value to 15%. Are there any other important regional settings that I might run, to make sure this constraint is valid in any case?

***It is important to mention that this constraint is key for speed performance, so increasing it might increase run times.

…creased to consider other region settings

Develop

emolinab added 30 commits April 14, 2020 14:48

Fetch develop magpie branch

677d78b

Script and realization without annuities (woa)

172c85d

corrections on not used variables

c281d85

Merge remote-tracking branch 'magpie_master/develop' into develop

cab5879

Script for develop runs

f55ad33

Last develop statusMerge remote-tracking branch 'magpie_master/develo…

b54e809

…p' into develop

fresh develop from master

2256903

Merge remote-tracking branch 'magpie_master/develop' into develop

b19bb49

Sticky merge current develop

afaf2f0

update not used variables in other factor realizations

ea4386d

start script

fffad22

bugs in the sticky start script

9a44bc5

inputs corrected in the script

5fc4fd7

Script bugs

e42d6e7

to skip c200 mixed realization

97576ea

Unavailable files for c400

8b6d1f5

Script

fe3e7a8

rev44 of isimip doesnt exist for c400

20bbf72

isimip rev 42 and 44

670eb1b

c400 resolution, old input

90700f7

High resoution parallel optimization. Trade -> exo

bb8b17e

libraries needed to read data missing FIXED

4c800a8

Update from develop branch Merge remote-tracking branch 'magpie_maste…

d6fdec5

…r/develop' into develop_sticky_merge

Sticky realization and annuities accumulation in the postsolve

1441f37

Merge remote-tracking branch 'magpie_master/develop' into develop_sti…

9cdf8d2

…cky_merge

Sticky Scripts

501c6c9

Sticky scripts including the lastest input .tgz

4e70925

Changes requested for pull request magpiemodel#186

9521a02

Test run

cd10c97

Changed name

37bdba1

Default test_runs.R script

6f48b0d

tscheypidi requested changes Jul 2, 2020

View reviewed changes

abhimishr added the enhancement New feature or request label Jul 2, 2020

emolinab added 3 commits July 2, 2020 17:10

Changes in current unreleased section

06cf509

typo

77ffd66

calculation of overall costs migrated to magpie4 postprocessing

5209625

tscheypidi approved these changes Jul 3, 2020

View reviewed changes

mishkos reviewed Jul 6, 2020

View reviewed changes

modules/38_factor_costs/sticky_feb18/declarations.gms Outdated Show resolved Hide resolved

mishkos reviewed Jul 6, 2020

View reviewed changes

modules/38_factor_costs/sticky_feb18/equations.gms Outdated Show resolved Hide resolved

emolinab added 3 commits July 6, 2020 11:51

Corrections Misko's reviews

8b539cb

test for corrections

ad62de2

Removed script for testing

03a2633

mishkos approved these changes Jul 7, 2020

View reviewed changes

bodirsky requested changes Jul 8, 2020

View reviewed changes

Merge remote-tracking branch 'magpie_master/develop' into develop_sti…

7a79c64

…cky_merge

emolinab added 3 commits July 22, 2020 15:25

Input for maximum fraction of GDP invested in agricultural capital in…

aa5e65c

…creased to consider other region settings

start script for test runs

9d0b6a0

bug fix

f8d9f8d

emolinab requested a review from bodirsky July 23, 2020 14:11

emolinab and others added 4 commits July 23, 2020 16:12

remove of sticky_test start script

60b4fb6

Merge pull request #2 from magpiemodel/develop

59c77ae

Develop

Merge pull request #3 from magpiemodel/develop

afb021e

Develop

Merge remote-tracking branch 'origin/develop' into develop_sticky_merge

59c2df4

bodirsky approved these changes Aug 12, 2020

View reviewed changes

bodirsky merged commit e2854ae into magpiemodel:develop Aug 12, 2020

bodirsky mentioned this pull request Aug 12, 2020

Revert "Develop-sticky merge" #208

Merged

abhimishr mentioned this pull request Aug 13, 2020

Revert "Revert "Develop-sticky merge"" #210

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop-sticky merge #199

Develop-sticky merge #199

emolinab commented Jul 2, 2020 •

edited

Loading

tscheypidi commented Jul 2, 2020

emolinab commented Jul 2, 2020

tscheypidi left a comment

tscheypidi Jul 2, 2020

emolinab Jul 2, 2020

tscheypidi Jul 2, 2020

emolinab Jul 2, 2020

tscheypidi Jul 2, 2020

tscheypidi Jul 2, 2020

emolinab Jul 2, 2020

tscheypidi Jul 3, 2020

emolinab Jul 3, 2020

bodirsky Jul 3, 2020

emolinab Jul 3, 2020

bodirsky Jul 3, 2020

tscheypidi left a comment

bodirsky left a comment

bodirsky Jul 3, 2020

emolinab commented Jul 22, 2020 •

edited

Loading


		scripts removed GP_final start script due to obsolescence

Develop-sticky merge #199

Develop-sticky merge #199

Conversation

emolinab commented Jul 2, 2020 • edited Loading

Description

Purpose of this PR

Performance loss/gain from the current default behavior

Type of change

How Has This Been Tested?

Default configuration additions/changes:

Checklist:

Special comments/warnings

tscheypidi commented Jul 2, 2020

emolinab commented Jul 2, 2020

tscheypidi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tscheypidi left a comment

Choose a reason for hiding this comment

bodirsky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emolinab commented Jul 22, 2020 • edited Loading

emolinab commented Jul 2, 2020 •

edited

Loading

emolinab commented Jul 22, 2020 •

edited

Loading