Bugfixes for model2netcdf.ED2()

.gitignore

@@ -102,3 +102,6 @@ contrib/modellauncher/modellauncher
 
 # don't checkin python venv used by CodeSpaces
 /.venv*
+
+# don't track project level .Rprofile
+.Rprofile

CHANGELOG.md

@@ -37,6 +37,8 @@ We are slowly change the license from NCSA opensource to BSD-3 to help with publ
 - rstudio was not working behind traefik.
 - plots now work in docker containers
 - when specifying diferent rstudio user, dev setup would mount pecan folder in wrong path.
+- bugs in `model2ncdf.ED2()` that were causing it to both error and also only
+convert data for a single PFT fixed (#1329, #2974, #2981)
 - Code cleanup in PEcAn.MA to protect against global namespace pollution (#2965, #2973; @nanu1605) 
 - Fixed vignettes and cleaned up internal warnings in PEcAn.DB (#2966, #2971; @nanu1605).
 - Updated unit conversions throughout PEcAn to use the `units` R package

CITATION.cff

@@ -117,6 +117,10 @@ authors:
   - given-names: Yan Zhao
   - affiliation: Uber
     given-names: Jing Xia
+  - given-names: Eric R. Scott
+    affiliation: University of Arizona
+    orcid: 'https://orcid.org/0000-0002-7430-7879'
+
 preferred-citation:
   type: article
   title: Facilitating feedbacks between field measurements and ecosystem models

base/utils/R/utils.R

@@ -27,44 +27,48 @@
 ##' @param silent logical: suppress log messages about missing variables?
 ##' @return ncvar based on MstMIP definition
 ##' @author Rob Kooper
-mstmipvar <- function(name, lat = NA, lon = NA, time = NA, nsoil = NA, silent = FALSE) {
+mstmipvar <- function(name, lat = NULL, lon = NULL, time = NULL, nsoil = NULL, silent = FALSE) {
   nc_var <- PEcAn.utils::standard_vars[PEcAn.utils::standard_vars$Variable.Name == name, ]
-  dims <- list()
 
   if (nrow(nc_var) == 0) {
     if (!silent) {
       PEcAn.logger::logger.info("Don't know about variable", name, " in standard_vars in PEcAn.utils")
     }
-    if (is.na(time)) {
-      time <- ncdf4::ncdim_def(name = "time", units = "days since 1900-01-01 00:00:00",
-                        vals = 1:365, calendar = "standard", unlim = TRUE)
+    if (is.null(time)) {
+      time <-
+        ncdf4::ncdim_def(
+          name = "time",
+          units = "days since 1900-01-01 00:00:00",
+          vals = 1:365,
+          calendar = "standard",
+          unlim = TRUE
+        )
     }
     return(ncdf4::ncvar_def(name, "", list(time), -999, name))
   }
-  for (i in 1:4) {
-    vd <- nc_var[[paste0("dim", i)]]
-    if (vd == "lon" && !is.na(lon)) {
-      dims[[length(dims) + 1]] <- lon
-    } else if (vd == "lat" && !is.na(lat)) {
-      dims[[length(dims) + 1]] <- lat
-    } else if (vd == "time" && !is.na(time)) {
-      dims[[length(dims) + 1]] <- time
-    } else if (vd == "nsoil" && !is.na(nsoil)) {
-      dims[[length(dims) + 1]] <- nsoil
-    } else if (is.na(vd)) {
-      # skip
-    } else {
-      if (!silent) {
-        PEcAn.logger::logger.info("Don't know dimension for", vd, "for variable", name)
-      }
+
+  var_dims <- nc_var[paste0("dim", 1:4)]
+  pos_dims <- c("lon", "lat", "time", "nsoil")
+
+  #check for missing dimensions
+  no_dims <- pos_dims[!var_dims %in% pos_dims]
+  if (!silent) {
+    if (length(no_dims) > 0) {
+      PEcAn.logger::logger.info("Don't know dimension(s)", no_dims, "for variable", name)
     }
   }
+
+  #replace dim names with values
+  var_dims <- var_dims[var_dims %in% pos_dims]
+  var_dims <- lapply(var_dims, function(x) eval(str2lang(x))) #converts character values in `var_dims` to corresponding R objects
+  dims <- var_dims[!sapply(var_dims, is.null)] #get rid of NULL elements
+
   ncvar <- ncdf4::ncvar_def(name, as.character(nc_var$Units), dims, -999)
   if (nc_var$Long.name != "na") {
     ncvar$longname <- as.character(nc_var$Long.name)
   }
   return(ncvar)
-} # mstimipvar
+} 
 
 
 #--------------------------------------------------------------------------------------------------#

base/utils/tests/testthat/test.utils.R

@@ -111,3 +111,53 @@ test_that("tryl returns FALSE if error, else true ", {
   expect_true(tryl(1+1))
   expect_true(!tryl(log("a")))
 })
+
+test_that("mstmipvar works with defaults", {
+  expect_s3_class(mstmipvar("NPP"), "ncvar4")
+})
+
+test_that("mstmipvar works with args specified", {
+  lat <- 
+    ncdf4::ncdim_def(
+      name = "lat",
+      longname = "station_latitude",
+      units = "degrees_north",
+      vals = 40,
+      unlim = FALSE
+    )
+  lon <-
+    ncdf4::ncdim_def(
+      name = "lon",
+      longname = "station_longitude",
+      units = "degrees_east",
+      vals = -80,
+      unlim = FALSE
+    )
+  time <-
+    ncdf4::ncdim_def(
+      name = "time",
+      units = "days since 1900-01-01 00:00:00",
+      vals = 30798:44765,
+      calendar = "standard",
+      unlim = TRUE
+    )
+  nsoil <- 
+    ncdf4::ncdim_def(
+      name = "SoilLayerMidpoint",
+      longname = "SoilLayerMidpoint",
+      units = "meters",
+      vals = c(-1.75,-1.25,-0.9,-0.7,-0.5,-0.3,-0.15,-0.075, 0),
+      unlim = FALSE
+    )
+
+  expect_s3_class(mstmipvar("NPP", lat = lat, lon = lon, time = time, nsoil = nsoil),
+                  "ncvar4")
+})
+
+# doesn't work because PEcAn.logger doesn't use message()
+# test_that("mstmipvar prints message with unknown var", {
+#   expect_message(
+#     mstmipvar("banana"),
+#     "Don't know about variable banana in standard_vars in PEcAn.utils"
+#   )
+# })

models/ed/.Rbuildignore

@@ -0,0 +1,4 @@
+^Dockerfile$
+^model_info\.json$
+^scripts$
+^data-raw$

models/ed/DESCRIPTION

@@ -18,6 +18,9 @@ Authors@R: c(person("Mike", "Dietze", role = c("aut", "cre"),
                      email = "[email protected]"),
              person("Alexey", "Shiklomanov", role = c("aut"),
                      email = "[email protected]"),
+             person("Eric R.", "Scott", role = c("ctb"),
+                     email = "[email protected]",
+                     comment = c(ORCID = "0000-0002-7430-7879")),
              person("University of Illinois, NCSA", role = c("cph")))
 Author: David LeBauer, Mike Dietze, Xiaohui Feng, Dan Wang, Carl
     Davidson, Rob Kooper, Shawn Serbin, Alexey Shiklomanov
@@ -28,6 +31,8 @@ Description: The Predictive Ecosystem Carbon Analyzer (PEcAn) is a scientific
     streamline the interaction between data and models, and to improve the
     efficacy of scientific investigation. This package provides functions to
     link the Ecosystem Demography Model, version 2, to PEcAn.
+Depends: 
+    R (>= 2.10)
 Imports:
     PEcAn.data.atmosphere,
     PEcAn.data.land,
@@ -49,11 +54,15 @@ Imports:
     utils,
     XML (>= 3.98-1.4)
 Suggests:
-    testthat (>= 1.0.2)
+    testthat (>= 1.0.2),
+    withr
+Additional_repositories: https://pecanproject.r-universe.dev/
 License: BSD_3_clause + file LICENSE
 Copyright: Authors
 LazyLoad: yes
-LazyData: FALSE
+LazyData: true
 Encoding: UTF-8
 RoxygenNote: 7.1.2
 Roxygen: list(markdown = TRUE)
+Config/testthat/edition: 2
+

models/ed/R/data.R

@@ -0,0 +1,12 @@
+#' Mapping of PEcAn PFT names to ED2 PFT numbers
+#'
+#' A dataset matching PEcAn PFT names to ED PFT numbers.
+#'
+#' @format A data frame with 73 rows and 2 variables:
+#' \describe{
+#'   \item{PEcAn}{PEcAn PFT names}
+#'   \item{ED}{ED2 PFT numbers}
+#'   ...
+#' }
+#' @source \url{https://github.com/EDmodel/ED2/wiki/Plant-functional-types}
+"pftmapping"