Merge pull request #1954 from orichters/develop

cleanup, tutorial update, testthat & renv version

.gitignore

@@ -19,6 +19,7 @@
 
 # Ignore everything in "input" folders
 input/
+!input/files
 !/scripts/input/
 
 # Ignore everything in root "output" folder

DESCRIPTION

@@ -54,7 +54,7 @@ Imports:
     nleqslv,
     optparse,
     piamenv (>= 0.6.0),
-    piamInterfaces (>= 0.34.1),
+    piamInterfaces (>= 0.40.9),
     piamPlotComparison (>= 0.0.10),
     piamutils,
     plotly,
@@ -65,7 +65,7 @@ Imports:
     readr,
     readxl,
     remind2 (>= 1.162.1),
-    renv,
+    renv (>= 1.0.7),
     reshape2,
     reticulate,
     rlang,
@@ -74,7 +74,7 @@ Imports:
     SPEI,
     stringr,
     terra,
-    testthat (>= 3.2.0),
+    testthat (>= 3.2.3),
     tibble,
     tidyr,
     tidyverse,

pull_request_template.md

@@ -21,11 +21,12 @@
 - [ ] I checked that the [in-code documentation](https://github.com/remindmodel/remind/blob/develop/main.gms#L120) is up-to-date
 - [ ] I adjusted the reporting in [`remind2`](https://github.com/pik-piam/remind2) where it was needed
 - [ ] I adjusted `forbiddenColumnNames` in [readCheckScenarioConfig.R](https://github.com/remindmodel/remind/blob/develop/scripts/start/readCheckScenarioConfig.R) in case the PR leads to deprecated switches
+- [ ] I checked the `log.txt` file of my runs for newly introduced summation, fixing or variable name errors
 - [ ] All automated model tests pass (`FAIL 0` in the output of `make test`)
 - [ ] The changelog `CHANGELOG.md` [has been updated correctly](https://gitlab.pik-potsdam.de/rse/rsewiki/-/wikis/Standards-for-Writing-a-Changelog)
 
 ## Further information (optional):
 
-* Test runs are here: 
+* Runs with these changes are here:
 * Comparison of results (what changes by this PR?): 
 

scripts/output/single/reporting.R

@@ -112,8 +112,10 @@ if (! is.null(magpie_reporting_file) && file.exists(magpie_reporting_file)) {
   piamutils::deletePlus(remind_reporting_file, writemif = TRUE)
 }
 
-# warn if duplicates in mif
-reportDuplicates(read.quitte(sub("\\.mif$", "_withoutPlus.mif", remind_reporting_file), check.duplicates = FALSE))
+# warn if duplicates in mif and incorrect spelling of variables
+mifcontent <- read.quitte(sub("\\.mif$", "_withoutPlus.mif", remind_reporting_file), check.duplicates = FALSE)
+reportDuplicates(mifcontent)
+invisible(piamInterfaces::checkVarNames(mifcontent))
 
 message("### end generation of mif files at ", round(Sys.time()))
 

scripts/start/prepare.R

@@ -440,31 +440,6 @@ prepare <- function() {
                                 list(c("vm_shBioFe.M", "!!vm_shBioFe.M")),
                                 list(c("q39_EqualSecShare_BioSyn.M", "!!q39_EqualSecShare_BioSyn.M")))
 
-    # OR: renamed for sectoral taxation
-    levs_manipulateThis <- c(levs_manipulateThis,
-                             list(c("vm_emiCO2_sector.L", "vm_emiCO2Sector.L")),
-                             list(c("v21_taxrevCO2_sector.L", "v21_taxrevCO2Sector.L")))
-    margs_manipulateThis <- c(margs_manipulateThis,
-                             list(c("vm_emiCO2_sector.M", "vm_emiCO2Sector.M")),
-                             list(c("v21_taxrevCO2_sector.M", "v21_taxrevCO2Sector.M")),
-                             list(c("q_emiCO2_sector.M", "q_emiCO2Sector.M")),
-                             list(c("q21_taxrevCO2_sector.M", "q21_taxrevCO2Sector.M")))
-    fixings_manipulateThis <- c(fixings_manipulateThis,
-                             list(c("vm_emiCO2_sector.FX", "vm_emiCO2Sector.FX")),
-                             list(c("v21_taxrevCO2_sector.FX", "v21_taxrevCO2Sector.FX")))
-
-    # OR: renamed in https://github.com/remindmodel/remind/pull/1495
-    levs_manipulateThis <- c(levs_manipulateThis,
-                             list(c("v_costInvTeDir.L", "vm_costInvTeDir.L")),
-                             list(c("v_costInvTeAdj.L", "vm_costInvTeAdj.L")))
-    margs_manipulateThis <- c(margs_manipulateThis,
-                             list(c("v_costInvTeDir.M", "vm_costInvTeDir.M")),
-                             list(c("v_costInvTeAdj.M", "vm_costInvTeAdj.M")))
-    fixings_manipulateThis <- c(fixings_manipulateThis,
-                             list(c("v_costInvTeDir.FX", "vm_costInvTeDir.FX")),
-                             list(c("v_costInvTeAdj.FX", "vm_costInvTeAdj.FX")))
-
-
     # renamed because of https://github.com/remindmodel/remind/pull/796
     manipulate_tradesets <- c(list(c("'gas_pipe'", "'pipe_gas'")),
                               list(c("'lng_liq'", "'termX_lng'")),
@@ -533,27 +508,6 @@ prepare <- function() {
                             list(c("v47_emiTargetMkt.FX", "!!v47_emiTargetMkt.FX")),
                             list(c("vm_taxrevimplEnergyBoundTax.FX", "!!vm_taxrevimplEnergyBoundTax.FX")))
 
-    # renamed because of https://github.com/remindmodel/remind/pull/1106
-    levs_manipulateThis <- c(levs_manipulateThis,
-                             list(c("v21_taxrevBioImport.L", "!!v21_taxrevBioImport.L")))
-    margs_manipulateThis <- c(margs_manipulateThis,
-                             list(c("v21_taxrevBioImport.M", "!!v21_taxrevBioImport.M")),
-                             list(c("q21_taxrevBioImport.M", "!!q21_taxrevBioImport.M")),
-                             list(c("q30_limitProdtoHist.M", "!!q30_limitProdtoHist.M")))
-    fixings_manipulateThis <- c(fixings_manipulateThis,
-                            list(c("v21_taxrevBioImport.FX", "!!v21_taxrevBioImport.FX")))
-
-    # renamed because of https://github.com/remindmodel/remind/pull/1128
-    levs_manipulateThis <- c(levs_manipulateThis,
-                             list(c("v_emiTeDetailMkt.L", "!!v_emiTeDetailMkt.L")),
-                             list(c("v_emiTeMkt.L", "!!v_emiTeMkt.L")))
-    margs_manipulateThis <- c(margs_manipulateThis,
-                             list(c("v_emiTeDetailMkt.M", "!!v_emiTeDetailMkt.M")),
-                             list(c("v_emiTeMkt.M", "!!v_emiTeMkt.M")))
-    fixings_manipulateThis <- c(fixings_manipulateThis,
-                            list(c("v_emiTeDetailMkt.FX", "!!v_emiTeDetailMkt.FX")),
-                             list(c("v_emiTeMkt.FX", "!!v_emiTeMkt.FX")))
-
     # Include fixings (levels) and marginals in full.gms at predefined position
     # in core/loop.gms.
     full_manipulateThis <- c(full_manipulateThis,

scripts/start/run.R

@@ -136,7 +136,10 @@ run <- function() {
                          config     = cfg,
                          runtime    = gams_runtime,
                          setup_info = lucode2::setup_info(),
-                         submit     = cfg$runstatistics)
+                         submit     = cfg$runstatistics,
+                         timeGAMSStart = timeGAMSStart,
+                         timeGAMSEnd   = timeGAMSEnd
+  )
 
   if (modelSummaryData[["stoprun"]]) {
     stop("GAMS did not complete its run, so stopping here:\n       No output is generated, no subsequent runs are started.\n",
@@ -265,10 +268,8 @@ run <- function() {
   timeOutputEnd <- Sys.time()
 
   # Save run statistics to local file
-  cat("\nSaving timeGAMSStart, timeGAMSEnd, timeOutputStart and timeOutputStart to runstatistics.rda\n")
+  cat("\nSaving timeOutputStart and timeOutputEnd to runstatistics.rda\n")
   lucode2::runstatistics(file           = paste0(cfg$results_folder, "/runstatistics.rda"),
-                       timeGAMSStart   = timeGAMSStart,
-                       timeGAMSEnd     = timeGAMSEnd,
                        timeOutputStart = timeOutputStart,
                        timeOutputEnd   = timeOutputEnd)
 

tests/testthat/test_01-SOF-EOF.R

@@ -5,7 +5,7 @@ test_that("all gms files have SOF and EOF statements", {
     SOF <- try(system(paste("grep -L '\\*\\*\\* *SOF.*$'", paste(files, collapse = " ")), intern = TRUE), silent = TRUE)
     EOF <- try(system(paste("grep -L '\\*\\*\\* *EOF.*$'", paste(files, collapse = " ")), intern = TRUE), silent = TRUE)
     missingSOFEOF <- sub("../../", "", sort(unique(c(SOF, EOF))), fixed = TRUE)
-    expect_equal(length(missingSOFEOF), 0)
+    expect_length(missingSOFEOF, 0)
     if (length(missingSOFEOF) > 0) {
       warning("These gms files lack SOF or EOF statements:\n", paste(missingSOFEOF, collapse = "\n"),
               "\nAdd '*** SOF' at the beginning and '*** EOF' at the end of the files and then run './scripts/utils/SOFEOF'")

tests/testthat/test_01-checkFixCfg.R

@@ -33,5 +33,5 @@ test_that("checkFixCfg works", {
   }
   expect_match(w, paste0(length(wrongsetting), " errors found"), all = FALSE, fixed = TRUE)
   expect_match(w, "Chosen RCP scenario 'apocalypse' might currently not be fully operational", all = FALSE, fixed = TRUE)
-  expect_equal(length(w), length(wrongsetting) + 2)
+  expect_length(w, length(wrongsetting) + 2)
 })

tests/testthat/test_01-manipulateConfig.R

@@ -35,7 +35,7 @@ test_that("manipulate config with default configuration does not change main.gms
             "Please file an issue in the gms package and try to adjust the code until the error goes away:\n",
             paste("-", removedgms, collapse = "\n"))
   }
-  expect_equal(length(removedgms), 0)
+  expect_length(removedgms, 0)
 
   # check for switches added to the new cfg
   addedgms <- setdiff(names(cfg_after$gms), names(cfg_init$gms))
@@ -44,7 +44,7 @@ test_that("manipulate config with default configuration does not change main.gms
             "Please file an issue in the gms package and try to adjust the code until the error goes away:\n",
             paste("-", addedgms, collapse = "\n"))
   }
-  expect_equal(length(addedgms), 0)
+  expect_length(addedgms, 0)
 
   # check for switches with different content between old and new cfg
   joinednames <- intersect(names(cfg_after$gms), names(cfg_init$gms))
@@ -53,7 +53,7 @@ test_that("manipulate config with default configuration does not change main.gms
     warning("After file manipulation, the following cfg$gms switches differ, see ", basename(tmpfile), ":\n",
             paste0("- ", contentdiff, ": ", unlist(cfg_init$gms[contentdiff]), " -> ", unlist(cfg_after$gms[contentdiff]), collapse = "\n"))
   }
-  expect_equal(length(contentdiff), 0)
+  expect_length(contentdiff, 0)
 
   # cleanup if no error found
   if (length(addedgms) + length(removedgms) + length(contentdiff) + length(diffresult) == 0) {

tutorials/01_GettingREMIND.md

@@ -11,7 +11,7 @@ REQUIREMENTS
 - input data
 - git
 - GAMS >= 39.1.0 with CONOPT license
-- R >= 4.0
+- R >= 4.0. We recommend R 4.3.2.
 - Windows only: RTools
 - LaTeX
 - pandoc
@@ -21,7 +21,13 @@ HOW TO INSTALL
 --------------
 
 To get the REMIND code first install git (<https://git-scm.com/downloads>).
-Then, to get the latest REMIND release:
+It is recommended to fork REMIND on your github user account.
+Then, on the PIK cluster, you can clone it using:
+```bash
+cloneremind https://github.com/yourusername/remind.git [remindfolder]
+```
+If you do not specify `[remindfolder]`, it uses `remind`.
+If you are not on the PIK cluster, to get the latest REMIND release:
 ```sh
 git clone -b master --filter=blob:limit=1m https://github.com/remindmodel/remind.git
 ```

tutorials/04_RunningREMINDandMAgPIE.md

@@ -23,11 +23,16 @@ David Klein (<[email protected]>)
 
 ### Clone the models
 
+If you work on the PIK cluster, first fork both https://github.com/magpiemodel/magpie.git and https://github.com/remindmodel/remind.git on your own github account, and then run
 ```bash
-git clone https://github.com/magpiemodel/magpie.git
-git clone --filter=blob:limit=1m https://github.com/remindmodel/remind.git
+clonerempie githubuser [remindfolder]
+```
+If `remindfolder` is not specified, it uses "remind".
+
+If you are not on the cluster, you can use the following to get a magpie folder within your remind folder:
+```bash
+git clone --filter=blob:limit=1m https://github.com/remindmodel/remind.git; cd remind; git clone https://github.com/magpiemodel/magpie.git
 ```
-Note: On the PIK cluster, use `cloneremind https://github.com/remindmodel/remind.git` instead of `git clone --filter…` to clone REMIND.
 
 ### Switch to relevant branchs
 

tutorials/13_Submit_to_IIASA_database.md

@@ -50,12 +50,13 @@ All the information printed to you during the run will also be present in the lo
 Check the logfile carefully for the variables that were omitted, failing summation checks etc.
 If you need information on a specific variable such as "Emi|CO2", you can run `piamInterfaces::variableInfo("Emi|CO2")` and it will provide a human-readable summary of the places this variable shows up in mappings and summation checks.
 Running `piamInterfaces::variableInfo("Emi|CO2", mapping = c("AR6", "mapping.csv"))` allows to compare your local mapping with the AR6 mapping with respect to this variable.
+On the PIK cluster, the script `variableinfo` is a shortcut, see `variableinfo --help`.
 
 If you specify `iiasatemplate`, the scripts will delete all the variables not in the template. This can be the reason that summation checks fail, simply because some of the variables that were reported by REMIND were omitted.
 
 Additionally, unit mismatches can cause the script to fail. In the past, IIASA has sometimes changed unit names to correct spelling mistakes or harmonize them.
-If there were unit mismatches where the units are identical, just spelled differently, you can add them to the named vector `identicalUnits` in [`piamInterfaces::checkFixUnits`](https://github.com/pik-piam/piamInterfaces/blob/master/R/checkFixUnits.R).
-So if the project template expects `Mt/yr`, but our mappings export it as `Mt/year`, add `c("Mt/yr", "Mt/year")` to the vector, and it will in the future not fail on this unit mismatch but correct it to what is required for the submission.
+If there were unit mismatches where the units are identical, just spelled differently, you can add them to the [`piamInterfaces::areUnitsIdentical()`](https://github.com/pik-piam/piamInterfaces/blob/master/R/areUnitsIdentical.R).
+So if the project template expects `Mt/yr`, but our mappings export it as `Mt/year`, add `c("Mt/yr", "Mt/year")`, and it will in the future not fail on this unit mismatch but correct it to what is required for the submission.
 Never use this mechanism if the units are not actually identical in their meaning.
 
 ## Step 4: upload file