Implement explicit scenario choice across package. #633

johanneskoch94 · 2025-02-14T10:39:48Z

Introduce a scenario argument to calcCapital, calcFeDemand, calcFeDemandBuildings, calcFloorspace, calcTheil, and calcEmiTarget. 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.
Move SSP2_NAV_all duplication to mrindustry.
Set NAs in CO2 prices from ExpertGuess to 0.
Drop unused sources argument in calcEmiTarget.
Drop unused scenario_proj argument in calcFE.
General refactoring.

!! 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 mrindustry.

LaviniaBaumstark · 2025-02-18T09:59:42Z

R/readStationary.R


-  scenarios <- list(SSPs  = paste0("SSP", 1:5),


Did you talk about read- and convertStationaryy with @fbenke-pik ?

No, ist there a something I should know @fbenke-pik ?

it will be removed at some point, but for now we need to keep it.

johanneskoch94 · 2025-02-19T17:46:20Z

Side note:
I see this warning popping up at in multiple calculations. It seems to come from something happening in calcOutput. How do I get rid of it?

WARNING:
~ Weight sum is 0, so cannot normalize and will return 0 for some aggregation targets. This changes the total sum of the magpie object! If this is really intended set zeroWeight = "allow", or "setNA" to return NA.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q · 2025-02-21T09:46:59Z

R/calcFEShares.R

-  fe_demand <- calcOutput("FEdemand", aggregate = FALSE)[, , "SSP1"]
+  fe_demand <- calcOutput("FEdemand", scenario = "SSP1", aggregate = FALSE)


Disables the madrat cache for this call. See pik-piam/mrindustry#30 (comment) and pik-piam/mrindustry#30 (comment). Should be fixed for the benefit of run times and disk space. And is probably an issue across related changes in other packages.

You mean it doesn't use a potential cache, in which the SSP1 scenario could be available right? I don't see a way around this without keeping a "default" set of scenarios, which is exactly what we want to get rid of. (Do you have any ideas?)

calcFEShares() is called by fullREMIND(), and that knows what scenarios are going to be calculated. Just pass the scenario [sic] parameter passed to e.g. calcFEdemand() to calcFEshares() too.

Ok! Thanks, will implement this right away.

Thank you for pointing this out, Michaja.

I just tested and noticed that madrat even does not recognize when you pass the same scenarios but in different order and will recalculate (so my point is we need to be aware of these things when passing additional arguments to heavy calc functions).

Introduce a scenario argument to calcCapital, calcFeDemand, calcFeDemandBuildings, calcFloorspace, calcTheil, and calcEmiTarget. 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. Move SSP2_NAV_all duplication to mrindustry. Set NAs in CO2 prices from ExpertGuess to 0. Drop unused sources argument in calcEmiTarget. Drop unused scenario_proj argument in calcFE. General refactoring.

robertsalzwedel

@johanneskoch94 asked me to approve this as a technicality.

fbenke-pik · 2025-02-28T10:05:43Z

R/readStationary.R

+#' @param x MAgPIE object returned from readStationary
+#' @param subset A string, or vector of string designating the entries for the scenario dimension. If possible,
+#'  passed to [mrdrivers::calcGDP()]. Unspecified scenarios receive SSP2 data.
+convertStationary <- function(x, subset) {


@johanneskoch94 moving read and convert into the same file is a big no for mrremind. Please don't do that.

fbenke-pik · 2025-02-28T10:14:49Z

R/readIIASA_subs_taxes.R

@@ -9,7 +9,6 @@
 #' pre final energy category \item \code{subsidies_bulk}: subsidy rate per final
 #' energy category \item \code{energy}: final energy quantities per category }
 #' @return magpie object of the IIASA_subs_taxes data
-#' @author Christoph Bertram


WHy did you remove the Author here? This is useful and important info, even if the person is no longer at PIK.

fbenke-pik · 2025-02-28T10:18:41Z

R/calcCO2Prices.R

  ceds <- ceds[, pmin(getYears(x), max(getYears(ceds))), ]
  getYears(ceds) <- getYears(x)

-  return(list(


We agreed as a team to not use that rule and switched off that linter. Therefore, I don't think it makes sense to go through legacy code to change it. Probably not worth changing it back now, either. So this is just a remark for future changes.

fbenke-pik · 2025-02-28T10:21:10Z

R/calcCapTarget.R

 #' @description The capacity targets (GW)  at regional level are produced from two different databases-
-#' UNFCCC_NDC database, an update of the Rogelj 2017 paper (see readme in inputdata), and REN21 Global Renewables report
-#' The UNFCCC_NDC capacity targets are further broken down to conditional and unconditional targets.
-#' @author Aman Malik, Oliver Richters


Please bring back all the @author tags. They are useful to evaluate who to ask for help and if the function is rather new or legacy code.

fbenke-pik · 2025-02-28T10:36:42Z

R/convertIIASA_subs_taxes.R

-#' a <- convertIIASA_subs_taxes(x)
-#' }
-#'
-convertIIASA_subs_taxes <- function(x, subtype) {


Should be brought back as an individual file

fbenke-pik · 2025-02-28T10:45:39Z

R/readGini.R

@@ -9,59 +9,26 @@
 #' described in the article. The country codes are the World Bank codes.
 #'
 #' @return magpie object of the Gini data
-#' @author Bjoern Soergel


Please bring back author

fbenke-pik · 2025-02-28T11:04:54Z

There are several files with only stylistic changes. In future PRs, please make a separate PR for these. When doing complex refactorings, as in calcCapTarget, readExpertGuess, readUNFCCC_NDC, run inputdata generation with those refactorings (and no expected changes in generated data) and ensure that all the inputdata is unchanged using madrat::compareData. I know you ran compareData for this PR as well, but the problem is that you mix actual data changes with refactorings that do not change data and therefore you cannot tell for sure if you did not break anything using compareData, unless you did advanced analysis per file.

fbenke-pik · 2025-02-28T12:01:21Z

Side note: I see this warning popping up at in multiple calculations. It seems to come from something happening in calcOutput. How do I get rid of it?
WARNING:
~ Weight sum is 0, so cannot normalize and will return 0 for some aggregation targets. This changes the total sum of the magpie object! If this is really intended set zeroWeight = "allow", or "setNA" to return NA.

This is a known issue, occurs when running some aggregations on magclass objects where NAs in weights (or possibly other reasons) cause the sum to be not equal to its parts. Needs to be addressed per case and is a task in the RSE backlog.

fbenke-pik · 2025-02-28T13:14:47Z

R/readUNFCCC_NDC.R

-#' @importFrom readxl read_xlsx
-#' @importFrom dplyr select
+#'  Emissions_2023_cond (or 2018/2021/2022 or uncond) for Emissions targets
+#' @param subset A string (or vector of strings) designating the scenario(s) to be returned.


Maybe add a remark that it is needed for the convert function, as it is not used directly in the read function

fbenke-pik · 2025-02-28T13:22:38Z

R/convertEdgeBuildings.R

-  }
-
-
+convertEdgeBuildings <- function(x, subtype, subset) {


readEdgeBuildings does not have the param 'subset`. Does it work then? If so, this is inconsistent with readUNFCCCC_NDR.R where you add the param to the read function (probably because you want to use it in the convert function).

johanneskoch94 requested a review from LaviniaBaumstark February 14, 2025 10:44

LaviniaBaumstark reviewed Feb 18, 2025

View reviewed changes

johanneskoch94 force-pushed the master branch from 8c78faf to 3ac80ff Compare February 19, 2025 17:27

johanneskoch94 changed the title ~~master~~ Implement explicit scenario choice across package. Feb 19, 2025

johanneskoch94 marked this pull request as ready for review February 19, 2025 17:31

johanneskoch94 force-pushed the master branch from 3ac80ff to aa97c52 Compare February 19, 2025 17:38

johanneskoch94 requested review from LaviniaBaumstark and fbenke-pik February 19, 2025 17:38

johanneskoch94 force-pushed the master branch from aa97c52 to d28b69d Compare February 19, 2025 17:40

johanneskoch94 mentioned this pull request Feb 19, 2025

Implement explicit scenario choice across package. pik-piam/mrindustry#29

Closed

johanneskoch94 requested a review from ricardarosemann February 20, 2025 13:15

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q reviewed Feb 21, 2025

View reviewed changes

johanneskoch94 force-pushed the master branch from 48b1ab0 to 340fe5f Compare February 21, 2025 15:50

johanneskoch94 force-pushed the master branch from 340fe5f to 2c06008 Compare February 21, 2025 16:01

johanneskoch94 added 2 commits February 21, 2025 17:12

Bump version

e8c136a

Fix bugs

75a0888

johanneskoch94 force-pushed the master branch from 35d3593 to 75a0888 Compare February 21, 2025 19:32

johanneskoch94 requested a review from robertsalzwedel February 21, 2025 19:36

robertsalzwedel approved these changes Feb 21, 2025

View reviewed changes

johanneskoch94 merged commit be1f255 into pik-piam:master Feb 21, 2025
1 check passed

fbenke-pik reviewed Feb 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement explicit scenario choice across package. #633

Implement explicit scenario choice across package. #633

johanneskoch94 commented Feb 14, 2025 •

edited

Loading

LaviniaBaumstark Feb 18, 2025

johanneskoch94 Feb 19, 2025

fbenke-pik Feb 19, 2025

johanneskoch94 commented Feb 19, 2025

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q Feb 21, 2025

johanneskoch94 Feb 21, 2025 •

edited

Loading

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q Feb 21, 2025

johanneskoch94 Feb 21, 2025

fbenke-pik Feb 28, 2025 •

edited

Loading

robertsalzwedel left a comment

fbenke-pik Feb 28, 2025

fbenke-pik Feb 28, 2025

fbenke-pik Feb 28, 2025 •

edited

Loading

fbenke-pik Feb 28, 2025

fbenke-pik Feb 28, 2025

fbenke-pik Feb 28, 2025

fbenke-pik commented Feb 28, 2025 •

edited

Loading

fbenke-pik commented Feb 28, 2025

fbenke-pik Feb 28, 2025

fbenke-pik Feb 28, 2025 •

edited

Loading

		fe_demand <- calcOutput("FEdemand", aggregate = FALSE)[, , "SSP1"]
		fe_demand <- calcOutput("FEdemand", scenario = "SSP1", aggregate = FALSE)

Implement explicit scenario choice across package. #633

Implement explicit scenario choice across package. #633

Conversation

johanneskoch94 commented Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanneskoch94 commented Feb 19, 2025

Choose a reason for hiding this comment

johanneskoch94 Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbenke-pik Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

robertsalzwedel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbenke-pik Feb 28, 2025 • edited Loading

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

fbenke-pik commented Feb 28, 2025 • edited Loading

fbenke-pik commented Feb 28, 2025

Choose a reason for hiding this comment

fbenke-pik Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

johanneskoch94 commented Feb 14, 2025 •

edited

Loading

johanneskoch94 Feb 21, 2025 •

edited

Loading

fbenke-pik Feb 28, 2025 •

edited

Loading

fbenke-pik Feb 28, 2025 •

edited

Loading

fbenke-pik commented Feb 28, 2025 •

edited

Loading

fbenke-pik Feb 28, 2025 •

edited

Loading