Add realization to 14_yields with regional and time-varying pasture Tau spillover values#882
Add realization to 14_yields with regional and time-varying pasture Tau spillover values#882alexkoberle wants to merge 5 commits into
Conversation
tscheypidi
left a comment
There was a problem hiding this comment.
GAMS implementation looks good, but there is no new input data revision. Please make sure that the new file is also part of the data inputs via madrat, otherwise the new realization will not work without additional input.
weindl
left a comment
There was a problem hiding this comment.
Thank you for the new realization. Looks good in general, only a few minor issues (see comments).
| p14_pyield_corr(t,i) = (f14_pyld_hist(t,i)/p14_pyield_LPJ_reg(t,i))$(sum(sameas(t_past,t),1) = 1) | ||
| + sum(t_past,(f14_pyld_hist(t_past,i)/(p14_pyield_LPJ_reg(t_past,i)+0.000001))$(ord(t_past)=card(t_past)))$(sum(sameas(t_past,t),1) <> 1); | ||
| i14_yields_calib(t,j,"pasture",w) = i14_yields_calib(t,j,"pasture",w) * sum(cell(i,j),p14_pyield_corr(t,i)); | ||
|
|
There was a problem hiding this comment.
These lines seem to be a code improvement that is not directly linked to the regional pasture spill over. Could you therefore also revise the original managementcalib_aug19 realization accordingly, such that the 2 realizations only differ regarding the new content (spill-over implementation)?
| *' | ||
| *' The parameter `f14_yld_past_switch(t_all,i)` scales the fraction of | ||
| *' crop-sector technological change (tau) that spills over to pasture yields, | ||
| *' and can vary across the 12 MAgPIE world regions and across 5-year model |
There was a problem hiding this comment.
As MAgPIE regions, time step length and simulation period can be adapted to specific applications, I recommend to remove the numbers.
| *' and can vary across the 12 MAgPIE world regions and across 5-year model | ||
| *' time steps from 1965 to 2150. A value of 0 implies no spillover; a value | ||
| *' of 1 implies full spillover equal to the crop-sector intensification rate. | ||
| *' The default input file populates all regions and time steps with 0.25, |
There was a problem hiding this comment.
I think that also the information about current default numbers should better be removed to avoid future inconsistencies when input data changes.
There was a problem hiding this comment.
Maybe you can still write, independently from the specific content of the input file, that the described set-up (all regions and time steps with 0.25) is in principal able to reproduce the behaviour of managementcalib_aug19.
🐦 Description of this PR 🐦
🔧 Checklist for PR creator 🔧
If a point is not applicable, check the checkbox anyway and write "non-applicable" next to the checkbox.
Label pull request from the label list.
Self-review own code
magpie4R library has been updated accordingly and backwards compatible where necessary.scenario_config.csvhas been updated accordingly (important ifdefault.cfghas been updated)Document changes
CHANGELOG.mdgoxygen::goxygen()and verify the modified code is properly documentedPerform test runs
Rscript start.R --> "compilation check"Rscript start.R --> "default"Rscript start.R --> "test runs"Reporting produces no errors and no new warnings
Get two approving reviews (at least one from RSE)
📉 Performance 📈
🚨 Checklist for reviewer 🚨
CHANGELOGis updated correctly