-
Notifications
You must be signed in to change notification settings - Fork 177
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
New EAT Lancet Diet Implementation #625
New EAT Lancet Diet Implementation #625
Conversation
… and decomposition runs
…would be zero to avoid divison by zero
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly minor changes. Please separate fruitvegnuts and roots switch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in general. For more details, see comments.
config/default.cfg
Outdated
# * options: BMK, FLX, PSC, VEG, VGN, FLX_hmilk, FLX_hredmeat | ||
cfg$gms$c15_EAT_scen <- "FLX" # def = FLX | ||
|
||
# * Subspecifications: Which commodities shall be included in the diet shift | ||
# * Sub-specifications: Which commodities shall be included in the diet shift | ||
# * towards cfg$gms$c15_EAT_scen ? Only selected (1) commodities will be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace "included in the diet shift towards cfg$gms$c15_EAT_scen" with
"included in the shift towards the selected scenario diet"
|
||
*' Extract fruits and vegetables that are part of others category | ||
* Note that starchy fruits are kept at the previously assigned maximum level | ||
* and their amount has been added to cassav_sp already. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"their amount has been added to cassav_sp already". Here I am not sure. I think their amount will be added, in line 551.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is part of p15_intake_detail_fruitveg and p15_intake_detailed_scen_fruitveg already
* (i15_rec_EATLancet(iso,"t_nutseeds","min") - i15_intake_detailed_scen_nuts(t,iso)) | ||
; | ||
|
||
*** ISABELLE: Please double-check whether I can remove the if condition from the nuts target equations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you mean the condition $(sum(kfo_ns2, p15_intake_detail(t,iso,kfo_ns2)) < (f15_rec_EATLancet(iso,"t_nutseeds","min") - i15_intake_detailed_scen_nuts(t,iso))?
I think it is a good idea to avoid that i15_intake_detailed_scen_nuts over-fulfill the nuts and seeds target. However, your solution would not work in the case, that even without the scaling-up of the i15_intake_detailed_scen_nuts, the sum of the old "others-nuts" plus remaining nuts would be above the target. In this case, the current implementation would correct this unnecessarily down.
Maybe one can first substract i15_intake_detailed_scen_nuts from sum(kfo_ns2, p15_intake_detail(t,iso,kfo_ns2)):
p15_intake_detailed_scen_target(t,iso,kfo_ns)$(...)
= p15_intake_detail(t,iso,kfo_ns) / sum(kfo_ns2, p15_intake_detail(t,iso,kfo_ns2))
* (sum(kfo_ns2, p15_intake_detail(t,iso,kfo_ns2)) - i15_intake_detailed_scen_nuts(t,iso))
;
and then apply the constraint:
p15_intake_detailed_scen_target(t,iso,kfo_ns)$(sum(kfo_ns2, p15_intake_detailed_scen_targett,iso,kfo_ns)) < (f15_rec_EATLancet(iso,"t_nutseeds","min") - i15_intake_detailed_scen_nuts(t,iso)) = p15_intake_detailed_scen_target(t,iso,kfo_ns) / sum(kfo_ns2, p15_intake_detailed_scen_target((t,iso,kfo_ns2))
* (f15_rec_EATLancet(iso,"t_nutseeds","min") - i15_intake_detailed_scen_nuts(t,iso))
;
Update changelog
Added reference to Springmann et al. 2018
added springmann reference
updated additional data
…ck to the coding etiquette
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super feli!
🐦 Description of this PR 🐦
cfg$gms$s15_exo_diet <- 3
, but the goal is to replace the old implementation by this one, i.e. delete code ofcfg$gms$s15_exo_diet <- 1
to have no parallel implementations. Note is included in default.cfg for the switch🔧 Checklist for PR creator 🔧
Label pull request from the label list.
Self-review own code
magpie4
R library has been updated accordingly and backwards compatible where necessary.scenario_config.csv
has been updated accordingly (important ifdefault.cfg
has been updated)Document changes
CHANGELOG.md
goxygen::goxygen()
and verify the modified code is properly documentedPerform test runs
Rscript start.R --> "compilation check"
Rscript start.R --> "test runs"
Rscript start.R --> "test runs"
📉 Performance changes 📈
🚨 Checklist for reviewer 🚨
CHANGELOG
is updated correctly