refactor calcFEdemand #458

fbenke-pik · 2024-01-04T13:37:52Z

Purpose:

Split up code in calcFEdemand by sectors, so that sector experts can adjust input data generation for their sector without messing with other sectors by accident.

Implementation Details

code of calcFEdemand is moved to calcFeDemandBuildings, calcFeDemandIndustry and calcFeDemandBuildings.
no longer needed subtypes were removed: ES, EsUeFe_in, EsUeFe_out, UE_for_Eff, FE_for_Eff
calcFEdemand no longer has any subtypes, calling it corresponds to formerly calling it with subtype FE
the subtypes FE_buildings, UE_buildings were migrated to calcFeDemandBuildings, as they only produce EDGE buildings data
the refactoring of code within the calc functions was kept to an absolute minimum necessary to spread code across functions per sector

Removed subtypes from calcFEdemand

ES, in consequence removed f29_esdemand.cs4r
EsUeFe_in, EsUeFe_out, in consequence removed p36_serviceInputs.cs4r, p36_serviceOutputs.cs4r
UE_for_Eff, FE_for_Eff, in consequence removed calcEnergyEffPaths, which creates output file f29_efficiency_growth.cs4

Refactoring of EDGE Buildings functions

renamed readEDGE, convertEDGE to readEdgeBuildings and convertEdgeBuildings
split reading in stationary data from readEDGE (subtype FE_stationary), as this is read in from a different source than EDGE Buildings
remove no longer needed subtypes from readEDGE: Capital, CapitalUnit, ES_buildings
rename subtypes in readEDGE: FE_buildings -> FE
removed subytpe CapitalUnit from calcCapital, as it is no longer needed in it's only calling function readEDGE -> therefore calcCapital no longer needs subtypes and former subtype Capital corresponds to current code; file f29_capitalUnitProjections.cs45 is no longer generated
remove EDGE buildings data from calcCapital, as it is no longer used. in consequence, f29_capitalQuantity.cs4r now has less variables, but one additional year (see below for details)

Depends on

remindmodel/remind#1509

Tests and Validation

expected file missing: f29_capitalUnitProjections.cs4r, f29_efficiency_growth.cs4r, f29_esdemand.cs4r, p36_serviceInputs.cs4r, p36_serviceOutputs.cs4r
expected file diffs: f29_capitalQuantity.cs4r, f_fedemand.cs4r (see below)
other than that, there should be not file differences in the data

inputdata-comparedata "rev6.60feDemandBaseline_2b1450bc_remind.tgz" "rev6.60feDemandCompare_2b1450bc_remind.tgz"
inputdata-comparedata "rev6.60feDemandBaseline_62eff8f7_remind.tgz" "rev6.60feDemandCompare_62eff8f7_remind.tgz"

differences in f_fedemand.cs4r

adds values for ueHDVt and ueLDVt for Low Energy, Campaigners and Navigate Scenarios due to a bugfix in this line

differences in f29_capitalQuantity.cs4r

removes

 [1] "gdp_SSP1.kapal"   "gdp_SSP2.kapal"   "gdp_SSP3.kapal"   "gdp_SSP4.kapal"   "gdp_SSP5.kapal"   "gdp_SSP1.kapsc"   "gdp_SSP2.kapsc"  
 [8] "gdp_SSP3.kapsc"   "gdp_SSP4.kapsc"   "gdp_SSP5.kapsc"   "gdp_SSP1.kaphc"   "gdp_SSP2.kaphc"   "gdp_SSP3.kaphc"   "gdp_SSP4.kaphc"  
[15] "gdp_SSP5.kaphc"   "gdp_SSP2EU.kapal" "gdp_SSP2EU.kapsc" "gdp_SSP2EU.kaphc" "gdp_SDP.kapal"    "gdp_SDP.kapsc"    "gdp_SDP.kaphc"   
[22] "gdp_SDP_EI.kapal" "gdp_SDP_EI.kapsc" "gdp_SDP_EI.kaphc" "gdp_SDP_RC.kapal" "gdp_SDP_RC.kapsc" "gdp_SDP_RC.kaphc" "gdp_SDP_MC.kapal"
[29] "gdp_SDP_MC.kapsc" "gdp_SDP_MC.kaphc"

adds year 1995

…Demand

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q · 2024-01-16T12:25:22Z

other than that, there should be not file differences in the data

And, is there?

fbenke-pik · 2024-01-16T13:09:55Z

No, there are no unexpected differences in input data, other than the ones I listed.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q

One immediate problem: there are now eight instances where we interact with the "fake" SSP2EU scenarios ([↑], [↑], [↑], [↑], [↑], [↑], [↑], [↑]).
I think this should be abstracted away (somehow), because I am currently struggling with extending it to yet more "fake" scenarios, and it is not pretty.

mellamoSimon · 2024-01-17T09:13:21Z

moin!
I just wanted to ask if you will consider streamlining these "fake SSP2EU" scenarios within this PR or if it's going to be postponed. And in both cases what is the timeline, if there's any? I need to finish some project work in the next week(s) that requires several "fake SSP2EU" scenarios. Thank you very much!

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q

@bs538, are you OK with killing ODYM_RECC? It was not used in the end, right?

bs538 · 2024-01-17T10:10:57Z

@bs538, are you OK with killing ODYM_RECC? It was not used in the end, right?

No, it wasn't used in the version for the final scenario runs. But the SHAPE papers are only about to be submitted, so if possible without major trouble I'd be in favour of keeping it in case we need to make modifications during the review.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q · 2024-01-17T10:27:32Z

One immediate problem: there are now eight instances where we interact with the "fake" SSP2EU scenarios ([↑], [↑], [↑], [↑], [↑], [↑], [↑], [↑]). I think this should be abstracted away (somehow), because I am currently struggling with extending it to yet more "fake" scenarios, and it is not pretty.

What's the general feeling about passing that information along via madrat::setConfig(), @fbenke-pik?

fbenke-pik · 2024-01-17T11:08:31Z

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q @mellamoSimon

Concerning "fake scenarios": thanks for pointing this out, Michaja. Indeed, there is potential for simplifying the code and I will do so.

Simón, for this PR my goal is to not change input data if possible. So in the end the scenarios will still be in the files generated as part of this PR. Will check together with the rest of RSE if we can get rid of some of it (maybe as part of a larger refactoring in REMIND) as a next step.

I will add further changes to this PR that postpone the generation of fake scenarios to the latest possible point, so that industry and transport no longer have to deal with it in the respective calcFEdemand functions, as it will only be done in calcFEDemand in the very end.

fbenke-pik · 2024-01-17T11:09:30Z

What's the general feeling about passing that information along via madrat::setConfig()

Haven't thought about it yet. What kind of info would you like to pass via setConfig?

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q · 2024-01-17T14:02:50Z

What's the general feeling about passing that information along via madrat::setConfig()

Haven't thought about it yet. What kind of info would you like to pass via setConfig?

setConfig(meaningful_option_name = list(
  gdp_SSP2EU = paste0("gdp_SSP2EU_", 
                      c(paste0("NAV_",   c("act", "tec", "ele", "lce", "all")),
                        paste0("CAMP_",  c("weak", "strong")),
                        paste0("ECEMF_", c("modEffInd", "IndLMD")),
                        paste0("GCS_",   c("IndLMD")))),
  gdp_SSP2 = "gdp_SSP2_lowEn"))

robinhasse · 2024-01-19T14:20:59Z

What's the general feeling about passing that information along via madrat::setConfig()

Isn't that information we can share with a mapping? Seems a bit cleaner to me than introducing a new key in the madrat configuration.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q · 2024-01-19T14:34:59Z

Kinda weird to read a file over and over again for the same bit of information. But than again, what's not weird with mrremind …

fbenke-pik · 2024-01-22T15:42:18Z

One immediate problem: there are now eight instances where we interact with the "fake" SSP2EU scenarios ([↑], [↑], [↑], [↑], [↑], [↑], [↑], [↑]). I think this should be abstracted away (somehow), because I am currently struggling with extending it to yet more "fake" scenarios, and it is not pretty.

Had a closer look, and unfortunately there is not much potential to simplify the code here (although I would have loved to get rid of some of it)

The assumption for this PR is that all these duplicate scenarios must still be present in the generated inputdata files for REMIND, using the same file structure. (If this was not the case, we could further simplify the code, but this should be done in a separate PR)

Why can't we get rid of it?

Cannot be removed from readEdgeBuildings, convertEdgeBuildings and calcFeDemandBuildings, because the input data from EDGE Buildings actually has "real" data for these scenarios as of now.
In theory, I could remove the duplication from readStationary, convertStationary and calcFeDemandIndustry; but then I would have to reintroduce it in calcFeDemandBuildings for stationary data and in calcFEdemand for the Industry and Transport data before merging it with the data from calcFeDemandBuildings, and it would not be much prettier than what we have right now.
readEDGETransport is a different routine existing alongside calcFEdemand and is currently refactored by Johanna, so out of scope for this PR

Also, I would like to highlight that this duplication has always been there, it was not introduced by this PR.

If we agreed on generating separate feDemand files for industry, transport and buildings that are read in individually by the respective modules in REMIND, we could definitely simplify the code and get rid of the fake scenarios. But this sth we would have to agree on collectively and implement separately (and it includes a potentially complex (?) refactoring on REMIND side).

fbenke-pik · 2024-01-22T15:47:36Z

What's the general feeling about passing that information along via madrat::setConfig()

Haven't thought about it yet. What kind of info would you like to pass via setConfig?
setConfig(meaningful_option_name = list(
  gdp_SSP2EU = paste0("gdp_SSP2EU_", 
                      c(paste0("NAV_",   c("act", "tec", "ele", "lce", "all")),
                        paste0("CAMP_",  c("weak", "strong")),
                        paste0("ECEMF_", c("modEffInd", "IndLMD")),
                        paste0("GCS_",   c("IndLMD")))),
  gdp_SSP2 = "gdp_SSP2_lowEn"))

Why would I want to change the config? From my understanding, inputdata generation always generates all the possible scenario data any REMIND run would want to use, no?

…Demand

johannah-pik · 2024-02-08T13:00:05Z

R/calcFeDemandTransport.R

@robertpietzcker I think this is only needed for complex - I will double check, but I think it can be deleted completely. Do you remember anything regarding this?

FYI there is a separate follow-up draft PR where we remove code related to transport
#468

fbenke-pik added 8 commits December 20, 2023 12:23

remove no longer needed subtypes in calcFEdemand

058ce26

move FE demand building subtypes to a separate calc function

3e27b59

add tool function

4a31751

remove subtype ES from calcFEdemand

b1dc5a1

remove subtype CapitalUnit from calcCapital

7b36af7

remove EDGE data from calcCapital

018c045

increment to dev version

3cffb1a

Split readEDGE into readEdgeBuildings and readStationary

f3fbdc6

fbenke-pik force-pushed the feDemand branch from 6dda8b6 to f3fbdc6 Compare January 5, 2024 16:13

add calcFeDemandIndustry

f642046

fbenke-pik force-pushed the feDemand branch from e25ddd3 to f642046 Compare January 8, 2024 15:31

remove subtypes FE_for_Eff and UE_for_Eff from calcFEdemand

9435aca

fbenke-pik force-pushed the feDemand branch 3 times, most recently from 63907ff to 17643ef Compare January 9, 2024 17:32

create calcFeDemandTransport

25091fa

fbenke-pik force-pushed the feDemand branch from 17643ef to 25091fa Compare January 9, 2024 17:43

enhance calcFeDemandTransport

0eea9c1

fbenke-pik force-pushed the feDemand branch 8 times, most recently from afe593d to 6e767cb Compare January 12, 2024 13:46

spread mapping accross calc functions

2cdd49e

fbenke-pik force-pushed the feDemand branch from 6e767cb to 2cdd49e Compare January 12, 2024 14:10

fbenke-pik added 2 commits January 12, 2024 16:20

move industry calculations

4ac3c28

Merge branch 'master' of https://github.com/pik-piam/mrremind into fe…

ff7fa12

…Demand

fbenke-pik requested review from johannah-pik and LaviniaBaumstark January 16, 2024 10:55

remove no longer used subtype in calls of calcFEdemand

3a0941a

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q reviewed Jan 16, 2024

View reviewed changes

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q reviewed Jan 17, 2024

View reviewed changes

add comments

54cf992

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q approved these changes Jan 23, 2024

View reviewed changes

fbenke-pik added 8 commits January 23, 2024 17:21

Merge branch 'master' of https://github.com/pik-piam/mrremind into fe…

151fdd5

…Demand

refactor EDGE Transport function calls in fullREMIND

5065369

increment version

8b4fbcd

Merge branch 'master' into feDemand

5cd422e

include NAV and CAMP scenarios in adjustments in convertStationary

c0759bb

increment version

4a6a898

Merge branch 'master' of https://github.com/pik-piam/mrremind into fe…

51f1b41

…Demand

increment version

91da4d5

fbenke-pik merged commit 6d65824 into pik-piam:master Feb 6, 2024
2 checks passed

fbenke-pik deleted the feDemand branch February 6, 2024 15:19

johannah-pik reviewed Feb 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor calcFEdemand #458

refactor calcFEdemand #458

fbenke-pik commented Jan 4, 2024 •

edited

Loading

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented Jan 16, 2024

fbenke-pik commented Jan 16, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q left a comment

mellamoSimon commented Jan 17, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q left a comment

bs538 commented Jan 17, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented Jan 17, 2024

fbenke-pik commented Jan 17, 2024

fbenke-pik commented Jan 17, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented Jan 17, 2024

robinhasse commented Jan 19, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented Jan 19, 2024

fbenke-pik commented Jan 22, 2024 •

edited

Loading

fbenke-pik commented Jan 22, 2024

johannah-pik Feb 8, 2024

fbenke-pik Feb 8, 2024

refactor calcFEdemand #458

refactor calcFEdemand #458

Conversation

fbenke-pik commented Jan 4, 2024 • edited Loading

Purpose:

Implementation Details

Removed subtypes from calcFEdemand

Refactoring of EDGE Buildings functions

Depends on

Tests and Validation

differences in f_fedemand.cs4r

differences in f29_capitalQuantity.cs4r

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented Jan 16, 2024

fbenke-pik commented Jan 16, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q left a comment

Choose a reason for hiding this comment

mellamoSimon commented Jan 17, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q left a comment

Choose a reason for hiding this comment

bs538 commented Jan 17, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented Jan 17, 2024

fbenke-pik commented Jan 17, 2024

fbenke-pik commented Jan 17, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented Jan 17, 2024

robinhasse commented Jan 19, 2024

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented Jan 19, 2024

fbenke-pik commented Jan 22, 2024 • edited Loading

fbenke-pik commented Jan 22, 2024

johannah-pik Feb 8, 2024

Choose a reason for hiding this comment

fbenke-pik Feb 8, 2024

Choose a reason for hiding this comment

fbenke-pik commented Jan 4, 2024 •

edited

Loading

fbenke-pik commented Jan 22, 2024 •

edited

Loading