From c7c485462eeac7cef90f93ec35099dc98f0ff577 Mon Sep 17 00:00:00 2001 From: orichters Date: Mon, 20 Jan 2025 08:04:32 +0100 Subject: [PATCH 1/5] cleanup, tutorial update, testthat & renv version --- DESCRIPTION | 6 +-- pull_request_template.md | 3 +- scripts/output/single/reporting.R | 6 ++- scripts/start/prepare.R | 46 ----------------------- tests/testthat/test_01-SOF-EOF.R | 2 +- tests/testthat/test_01-checkFixCfg.R | 2 +- tests/testthat/test_01-manipulateConfig.R | 6 +-- tutorials/01_GettingREMIND.md | 10 ++++- tutorials/04_RunningREMINDandMAgPIE.md | 11 ++++-- 9 files changed, 30 insertions(+), 62 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 141236d28..4560a7dab 100644 --- a/DESCRIPTION +++ b/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, diff --git a/pull_request_template.md b/pull_request_template.md index 71349877e..783c0c472 100644 --- a/pull_request_template.md +++ b/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?): diff --git a/scripts/output/single/reporting.R b/scripts/output/single/reporting.R index 2689c64e2..f3a42f346 100644 --- a/scripts/output/single/reporting.R +++ b/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())) diff --git a/scripts/start/prepare.R b/scripts/start/prepare.R index 945288f44..ea993319b 100644 --- a/scripts/start/prepare.R +++ b/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, diff --git a/tests/testthat/test_01-SOF-EOF.R b/tests/testthat/test_01-SOF-EOF.R index 934ce7f63..a0ac134bb 100644 --- a/tests/testthat/test_01-SOF-EOF.R +++ b/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'") diff --git a/tests/testthat/test_01-checkFixCfg.R b/tests/testthat/test_01-checkFixCfg.R index 3d4b51479..3770c3673 100644 --- a/tests/testthat/test_01-checkFixCfg.R +++ b/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) }) diff --git a/tests/testthat/test_01-manipulateConfig.R b/tests/testthat/test_01-manipulateConfig.R index 7e3793a43..0da1832ae 100644 --- a/tests/testthat/test_01-manipulateConfig.R +++ b/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) { diff --git a/tutorials/01_GettingREMIND.md b/tutorials/01_GettingREMIND.md index 97705a9f3..ef89b6e2e 100644 --- a/tutorials/01_GettingREMIND.md +++ b/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 (). -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 ``` diff --git a/tutorials/04_RunningREMINDandMAgPIE.md b/tutorials/04_RunningREMINDandMAgPIE.md index 462f55dc7..80b4a2eb7 100644 --- a/tutorials/04_RunningREMINDandMAgPIE.md +++ b/tutorials/04_RunningREMINDandMAgPIE.md @@ -23,11 +23,16 @@ David Klein () ### 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 From 2973161d17ae45c79ad10b31b8d15ea97c9069e1 Mon Sep 17 00:00:00 2001 From: orichters Date: Mon, 20 Jan 2025 15:33:08 +0100 Subject: [PATCH 2/5] write timeGAMSStart and End before eventual stop() call --- scripts/start/run.R | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/scripts/start/run.R b/scripts/start/run.R index f07725cae..a637c87d4 100644 --- a/scripts/start/run.R +++ b/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) From 311f47f5ef51ad696d61f1b41b9a92657b6a6bac Mon Sep 17 00:00:00 2001 From: orichters Date: Tue, 21 Jan 2025 16:26:00 +0100 Subject: [PATCH 3/5] tutorial update --- tutorials/13_Submit_to_IIASA_database.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tutorials/13_Submit_to_IIASA_database.md b/tutorials/13_Submit_to_IIASA_database.md index 8fefdadff..b7df3c7d7 100644 --- a/tutorials/13_Submit_to_IIASA_database.md +++ b/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 From 3fa63876c1f30938c71c8ff969c42e6cf2e9019e Mon Sep 17 00:00:00 2001 From: orichters Date: Wed, 22 Jan 2025 08:24:52 +0100 Subject: [PATCH 4/5] do not ignore input/files file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index ea3708d2f..6eb3a3c66 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ # Ignore everything in "input" folders input/ +!input/files !/scripts/input/ # Ignore everything in root "output" folder From d294a58fee2aa32bcb8e85f2b98e68896fe9735d Mon Sep 17 00:00:00 2001 From: orichters Date: Thu, 23 Jan 2025 10:31:34 +0100 Subject: [PATCH 5/5] ignoreFiles.R is superfluous and its use would be counterproductive --- ignoreFiles.R | 31 ------------------------------- 1 file changed, 31 deletions(-) delete mode 100644 ignoreFiles.R diff --git a/ignoreFiles.R b/ignoreFiles.R deleted file mode 100644 index d4f6874dd..000000000 --- a/ignoreFiles.R +++ /dev/null @@ -1,31 +0,0 @@ -# | (C) 2006-2024 Potsdam Institute for Climate Impact Research (PIK) -# | authors, and contributors see CITATION.cff file. This file is part -# | of REMIND and licensed under AGPL-3.0-or-later. Under Section 7 of -# | AGPL-3.0, you are granted additional permissions described in the -# | REMIND License Exception, version 1.0 (see LICENSE file). -# | Contact: remind@pik-potsdam.de -files_path = list.files(".",recursive = T,pattern = "files$") -paths_input = NULL - -for (.file in files_path){ - content_file = readLines(.file) - .path_file = gsub("/files$","",.file) - .paths_input = paste0(.path_file,"/",content_file) - paths_input = c(paths_input, .paths_input) -} - -#Git ignore -write(x = c(".*.un~", - ".*.swp", - "input/", - "output/*", - "main.lst", - "doc/doc.rds", - "doc/documentation.*", - "doc/goxygen_pdflatex.log", - "doc/html/", - "doc/markdown/", - "Rplots.pdf", - paths_input), - ".gitignore") -