Implement explicit scenario choice across package. #29
Conversation
johanneskoch94
force-pushed
the
main
branch
2 times, most recently
from
9b05321 to
bfddd91
Compare
R/industry_subsectors_specific.R
Outdated
There was a problem hiding this comment.
I was wondering what the benefit of the "switchboard - with - functions" implementation is, vs a switch implementation, as done here? Is there some mechanic I'm overlooking, or is it simply a design choice? If so, what do you think of using switch instead?
johanneskoch94
changed the title
Introduce a scenario argument to calcFeDemandIndustry, calcIndustry_EEK and calcIndustry_Value_Added. The larger picture is the attempt to step away from a default set of scenarios. The choice of scenarios should be explicit. In the case of the input data pipeline, it is made in fullREMIND and then passed on to the different calcOutput functions. Hopefully this will help us control/drop/introduce new scenarios more easily in the future. Add placeholder assumptions in inst data for the SSP2IndiaDEA scenarios. Move SSP2_NAV_all duplication over from mrremind. Remove naming argument in mrdrivers functions. Delete convertUSGS, fixes pik-piam#14. General refactoring.
There was a problem hiding this comment.
This is just a placeholder - but I'm not at all sure if this is the right place to even set up placeholder assumptions for new scenarios.
| if ("SSP2_NAV_all" %in% scenario) { | ||
| remind <- mbind(remind, setItems(remind[, , "SSP2"], 3.1, "SSP2_NAV_all")) | ||
| } |
There was a problem hiding this comment.
A better solution for the SSP2_NAV_all scenario should be implemented at some point, but I couldn't figure out where to do it. (I copied this over from mrremind).
| remind_scenarios <- c( | ||
| paste0("SSP", c(1:5, "2_lowEn", "2_highDemDEU", "2IndiaHigh", "2IndiaMedium")), | ||
| paste0("SDP", c("", "_EI", "_RC", "_MC")) | ||
| ) |
There was a problem hiding this comment.
This is not ideal, but it's the best I could manage. Ideally we would use the scenario argument directly, but I did not manage to get the function to work, if not all the scenarios are included. If someone else could take over, that would be amazing! For now, the function computes the demand for all the scenarios in remind_scenarios, and then filters to match the scenarios in scenario at the end.
johanneskoch94
requested review from
JakobBD,
LaviniaBaumstark and
0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
|
I am not going to review, or approve, a pull request where you modify swathes code that do not impinge on the things you need to modify in any way, because you need to scratch some itch of yours. |
|
Yeah, I was afraid of this. I'll set up a new PR and remove the refactoring. |
|
Regarding convertUSGS, it might still be needed by @bennet21. |
|
Clean version of most important changes here: #30 |
!! Still waiting for my input data test to run through (/p/tmp/jokoch/pre-processing, output at /p/projects/rd3mod/inputdata/output_1.27/rev7.22johannes_test7_62eff8f7_remind) but tagged in the reviewers so that the revision can start.
Connected to this PR in mrremind.