PecanProject / pecan

The Predictive Ecosystem Analyzer (PEcAn) is an integrated ecological bioinformatics toolbox.
www.pecanproject.org
Other
202 stars 234 forks source link

`model2netcdf.ED2()` output isn't grouped by PFT #2997

Closed Aariq closed 1 year ago

Aariq commented 2 years ago

Bug Description

The .nc files resulting from conversion from ED2 output have variables that could be split up by PFT but aren't. For example NPP in the .nc file only has the dimensions lat, lon, and time but it is possible to split up by PFT in the .h5 files produced by ED2. The only variables that have PFT as a dimension are DBH, rate of change of DBH and plant density.

To Reproduce

devtools::load_all("models/ed")
outdir <- tempfile()
unzip("models/ed/tests/testthat/data/ed2_run_output.zip", exdir = outdir)
file.copy("models/ed/tests/testthat/data/pecan_checked.xml", file.path(outdir, "pecan_checked.xml"))

settings <- PEcAn.settings::read.settings(file.path(outdir, "pecan_checked.xml"))
settings$outdir <- outdir

model2netcdf.ED2(settings = settings)

library(ncdf4)
x <- nc_open(list.files(outdir, pattern = "*.nc$", full.names = TRUE))

x

Expected behavior

It would be helpful for at least the option to split variables up by PFT. One way to do this to ensure backwards compatibility would be to add an argument by_pft = FALSE that changes nothing if kept default, but adds PFT as a dimension wherever possible when TRUE

Aariq commented 2 years ago

Looks like maybe this was a planned feature already: https://github.com/PecanProject/pecan/blob/01b370430304dfd965aa7b31e15f209ecc78775e/models/ed/R/model2netcdf.ED2.R#L903

istfer commented 2 years ago

FWIW, the older version of the read-E-files function used to be able to group outputs in the -E- files by PFT. We had a discussion in the slack channel a while ago, here is my last comment there:

but the current version is broken, and unfortunately I'm not familiar with the new changes, even the function arguments have changed, before pfts was pft_names and it was passed via upstream functions as a vector

But now the code is changed to behave differently