From f10b2d33ed3f9656883d6f4c395d3122317eafec Mon Sep 17 00:00:00 2001 From: "Eric R. Scott" Date: Thu, 22 Dec 2022 13:57:38 -0500 Subject: [PATCH 1/4] `data()` doesn't work with internal datasets --- models/ed/R/write.configs.ed.R | 22 ++-- .../ed/tests/testthat/test.write.configs.ed.R | 115 +++++++++++------- 2 files changed, 87 insertions(+), 50 deletions(-) diff --git a/models/ed/R/write.configs.ed.R b/models/ed/R/write.configs.ed.R index 805c10c4c8d..95e3be8368b 100644 --- a/models/ed/R/write.configs.ed.R +++ b/models/ed/R/write.configs.ed.R @@ -443,18 +443,22 @@ remove.config.ED2 <- function(main.outdir = settings$outdir, settings) { #' @author David LeBauer, Shawn Serbin, Carl Davidson, Alexey Shiklomanov write.config.xml.ED2 <- function(settings, trait.values, defaults = settings$constants) { - ## Find history file TODO this should come from the database - ed2_package_data <- data(package="PEcAn.ED2", envir = environment()) - histfile <- paste0("history.r", settings$model$revision) # set history file name to look for in ed2_package_data - if (histfile %in% ed2_package_data$results[, "Item"]) { + # TODO this should come from the database + + if(settings$model$revision %in% c("46", "81", "82", "85", "git")) { + histfile <- paste0("history.r", settings$model$revision) PEcAn.logger::logger.debug(paste0("--- Using ED2 History File: ", histfile)) - data(list=histfile, package = 'PEcAn.ED2', envir = environment()) - edhistory <- get(histfile) + edhistory <- + switch(settings$model$revision, + "46" = PEcAn.ED2:::history.r46, + "81" = PEcAn.ED2:::history.r81, + "82" = PEcAn.ED2:::history.r82, + "85" = PEcAn.ED2:::history.r85, + "git" = PEcAn.ED2:::history.rgit + ) } else { PEcAn.logger::logger.debug("--- Using Generic ED2 History File: history.csv") - histfile <- "history" - data(list=histfile, package = 'PEcAn.ED2', envir = environment()) - edhistory <- get(histfile) + edhistory <- PEcAn.ED2:::history } edtraits <- names(edhistory) diff --git a/models/ed/tests/testthat/test.write.configs.ed.R b/models/ed/tests/testthat/test.write.configs.ed.R index 50d68af26b5..d1abe66ba48 100644 --- a/models/ed/tests/testthat/test.write.configs.ed.R +++ b/models/ed/tests/testthat/test.write.configs.ed.R @@ -6,6 +6,49 @@ ## # which accompanies this distribution, and is available at ## # http://opensource.ncsa.illinois.edu/license.html ## #------------------------------------------------------------------------------- +# dummy trait values +trait.values <- + list( + SetariaWT = structure( + list( + mort2 = 19.9551305310619, + growth_resp_factor = 0.271418338660799, + leaf_turnover_rate = 4.08633744111248, + leaf_width = 4.16095405673501, + nonlocal_dispersal = 0.208572992916628, + fineroot2leaf = 2.25017242716454, + root_turnover_rate = 0.527523622000746, + seedling_mortality = 0.949939872416094, + stomatal_slope = 4.07086860946459, + quantum_efficiency = 0.0565042189665225, + Vcmax = 22.3047025944851, + r_fract = 0.313812660341759, + cuticular_cond = 12992.9906222683, + root_respiration_rate = 5.48040042748477, + Vm_low_temp = 10.0000004842057, + SLA = 40.1495401375622 + ), + row.names = "50", + class = "data.frame" + ), + env = structure( + list(), + .Names = character(0), + row.names = "NA", + class = "data.frame" + ) + ) + +defaults <- + list( + pft = list( + name = "SetariaWT", + ed2_pft_number = "1", + outdir = "/home/ericrscott/model-vignettes/ED2/testoutput/two_pfts/outdir//pft/SetariaWT", + posteriorid = 9000001416 + ) + ) + test_that("convert.samples.ED works as expected",{ testthat::local_edition(3) expect_equal(convert.samples.ED(c("Vcmax" = 1))[["Vcmax"]], @@ -48,47 +91,7 @@ test_that("New ED2IN tags get added at bottom of file", { #3. add arbitrary ed2in_tag to settings list settings$model$ed2in_tags$NEW_TAG <- "0" #4. run write.config.ED2 - trait.values <- - list( - SetariaWT = structure( - list( - mort2 = 19.9551305310619, - growth_resp_factor = 0.271418338660799, - leaf_turnover_rate = 4.08633744111248, - leaf_width = 4.16095405673501, - nonlocal_dispersal = 0.208572992916628, - fineroot2leaf = 2.25017242716454, - root_turnover_rate = 0.527523622000746, - seedling_mortality = 0.949939872416094, - stomatal_slope = 4.07086860946459, - quantum_efficiency = 0.0565042189665225, - Vcmax = 22.3047025944851, - r_fract = 0.313812660341759, - cuticular_cond = 12992.9906222683, - root_respiration_rate = 5.48040042748477, - Vm_low_temp = 10.0000004842057, - SLA = 40.1495401375622 - ), - row.names = "50", - class = "data.frame" - ), - env = structure( - list(), - .Names = character(0), - row.names = "NA", - class = "data.frame" - ) - ) - - defaults <- - list( - pft = list( - name = "SetariaWT", - ed2_pft_number = "1", - outdir = "/home/ericrscott/model-vignettes/ED2/testoutput/two_pfts/outdir//pft/SetariaWT", - posteriorid = 9000001416 - ) - ) + x <- capture.output( write.config.ED2( trait.values = trait.values, @@ -120,6 +123,36 @@ test_that("New ED2IN tags get added at bottom of file", { }) +test_that("write.config.xml.ED2() uses correct history file", { + #1. read in pecan.xml in data/pecan_checked.xml + settings <- PEcAn.settings::read.settings("data/pecan_checked.xml") + #for debugging: + # settings <- PEcAn.settings::read.settings("models/ed/tests/testthat/data/pecan_checked.xml") + + #2. Set rundir to tempdir + rundir <- tempfile() + dir.create(rundir) + on.exit(unlink(rundir, recursive = TRUE)) + settings$rundir <- rundir + run.id <- "ENS-00001-76" + dir.create(file.path(rundir, run.id)) + #3. set revision to 81 + settings$model$revision <- "81" + + x <- capture.output( + write.config.xml.ED2( + settings = settings, + trait.values = trait.values, + defaults = defaults + ), + type = "message" + ) + + expect_true(any(stringr::str_detect(x, "history.r81"))) + +}) + + ## test_that("remove.configs.ED2 works with remote host",{ ## settings <- list(outdir = "/tmp/", ## run = list(host = list(name = "ebi-cluster.igb.illinois.edu", From cb89de940a5adf10dcf4ccb92bc6a0b145fae950 Mon Sep 17 00:00:00 2001 From: "Eric R. Scott" Date: Thu, 22 Dec 2022 13:59:12 -0500 Subject: [PATCH 2/4] update news --- CHANGELOG.md | 1 + models/ed/NEWS.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1102f1342ab..458342b0286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ convert data for a single PFT fixed (#1329, #2974, #2981) the cdo_setup argument in the template job file. In detail, people will need to specify cdosetup = "module load cdo/2.0.6" in the host section. More details are in the Create_Multi_settings.R script. (#3052) +- write.config.xml.ED2() wasn't using the tag in settings correctly (#3080) ### Changed diff --git a/models/ed/NEWS.md b/models/ed/NEWS.md index 50d3b11fa6c..04e4a89b374 100644 --- a/models/ed/NEWS.md +++ b/models/ed/NEWS.md @@ -1,6 +1,7 @@ # PEcAn.ED2 (development version) * Added optional `process_partial` argument to `model2netcdf.ED2()` to allow it to process existing output from failed runs. +* write.config.xml.ED2() wasn't using the tag in settings correctly (#3080) # PEcAn.ED2 1.7.2 From 39b1da480df435f32358de6a2fd0749c0c7bbb3b Mon Sep 17 00:00:00 2001 From: "Eric R. Scott" Date: Fri, 23 Dec 2022 11:53:21 -0500 Subject: [PATCH 3/4] whoops, guess I was wrong about how to refer to internal data --- models/ed/R/write.configs.ed.R | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/models/ed/R/write.configs.ed.R b/models/ed/R/write.configs.ed.R index 95e3be8368b..ea15d94f18d 100644 --- a/models/ed/R/write.configs.ed.R +++ b/models/ed/R/write.configs.ed.R @@ -450,15 +450,15 @@ write.config.xml.ED2 <- function(settings, trait.values, defaults = settings$con PEcAn.logger::logger.debug(paste0("--- Using ED2 History File: ", histfile)) edhistory <- switch(settings$model$revision, - "46" = PEcAn.ED2:::history.r46, - "81" = PEcAn.ED2:::history.r81, - "82" = PEcAn.ED2:::history.r82, - "85" = PEcAn.ED2:::history.r85, - "git" = PEcAn.ED2:::history.rgit + "46" = history.r46, + "81" = history.r81, + "82" = history.r82, + "85" = history.r85, + "git" = history.rgit ) } else { PEcAn.logger::logger.debug("--- Using Generic ED2 History File: history.csv") - edhistory <- PEcAn.ED2:::history + edhistory <- history } edtraits <- names(edhistory) From 6cb6f9c552e7fd8318c8d5c5d25a4202644076c6 Mon Sep 17 00:00:00 2001 From: "Eric R. Scott" Date: Tue, 10 Jan 2023 15:14:12 -0500 Subject: [PATCH 4/4] different, more scalable strategy for matching the correct history file. Added comments to explain what's going on. --- models/ed/R/write.configs.ed.R | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/models/ed/R/write.configs.ed.R b/models/ed/R/write.configs.ed.R index ea15d94f18d..9c4645bb5d9 100644 --- a/models/ed/R/write.configs.ed.R +++ b/models/ed/R/write.configs.ed.R @@ -445,20 +445,26 @@ write.config.xml.ED2 <- function(settings, trait.values, defaults = settings$con # TODO this should come from the database - if(settings$model$revision %in% c("46", "81", "82", "85", "git")) { - histfile <- paste0("history.r", settings$model$revision) - PEcAn.logger::logger.debug(paste0("--- Using ED2 History File: ", histfile)) - edhistory <- - switch(settings$model$revision, - "46" = history.r46, - "81" = history.r81, - "82" = history.r82, - "85" = history.r85, - "git" = history.rgit - ) + # Internal data sets stored in sysdata.RDA are used to override defaults in + # config.xml. This code looks for a history dataset that matches the + # "revision" number for ED2 set in settings (e.g. PEcAn.ED2:::history.r85) and + # if it doesn't find it, it uses a generic file (PEcAn.ED2:::history). To add + # a new history file, add the .csv file to models/ed/data-raw and run the + # sysdata.R script in that folder + + if(is.null(settings$model$revision)) { + PEcAn.logger::logger.debug("--- Using Generic ED2 History File") + edhistory <- history } else { - PEcAn.logger::logger.debug("--- Using Generic ED2 History File: history.csv") + histfile <- paste0("history.r", settings$model$revision) + edhistory <- try(eval(str2lang(histfile)), silent = TRUE) + } + + if(inherits(edhistory, "try-error")) { + PEcAn.logger::logger.debug("--- Using Generic ED2 History File") edhistory <- history + } else { + PEcAn.logger::logger.debug(paste0("--- Using ED2 History File: ", histfile)) } edtraits <- names(edhistory)