Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

model2netcdf.ED2 Fails if output files missing "AVG_SOIL_TEMP" aka "Soiltemp" #2623

Closed
mccabete opened this issue Jun 2, 2020 · 3 comments · Fixed by #2636
Closed

model2netcdf.ED2 Fails if output files missing "AVG_SOIL_TEMP" aka "Soiltemp" #2623

mccabete opened this issue Jun 2, 2020 · 3 comments · Fixed by #2636

Comments

@mccabete
Copy link
Contributor

mccabete commented Jun 2, 2020

Bug Description

model2netcdf.ED2 fails if -T- file is missing the variable "AVG_SOIL_TEMP", known as "Soiltemp" in the pecan standard. It fails specifically at this line.

To Reproduce

Steps to reproduce the behavior:

  1. Run met2model.ED2() on an ED2 generated -T- file that has no "AVG_SOIL_TEMP" data.
  2. See error

Expected behavior

The function's assigning Soiltemp the value -999 would mean that the .nc file it produces would have "missing values" of -999 for Soiltemp.

Error message

2020-06-02 16:00:40 WARN   [model2netcdf.ED2.R#278: PEcAn.logger::logger.warn] : 
   Could not find AVG_SOIL_TEMP in ed hdf5 output. 
Error in array(0, ncol(soiltemp)) : 'dims' cannot be of length 0

Machine (please complete the following information):

  • Server BU test-pecan, Tess' personal build. Up to date with develop.

Additional context

I tried an if(soiltemp != -999){} around the code chunk from line 520 to line 579, but it messed up the dimensions during the write-out step.

@mccabete mccabete changed the title model2netcdf.ED2 Fails if missing "AVG_SOIL_TEMP" aka "Soiltemp" model2netcdf.ED2 Fails if output files missing "AVG_SOIL_TEMP" aka "Soiltemp" Jun 2, 2020
@mccabete
Copy link
Contributor Author

This error is dependent on the output files being written out by the version of ED2 one is using. In my personal branch, I modified (this section)[https://github.com/PecanProject/pecan/blob/develop/models/ed/R/model2netcdf.ED2.R#L285] to check for FMEAN_SOIL_TEMP_PY instead of average values.

CheckED2Version <- function(nc) {
    if ("FMEAN_BDEAD_PY" %in% names(nc$var)) {
      return("Git")
    }
    if ("FMEAN_SOIL_TEMP_PY" %in% names(nc$var)){
      return("Tess")
    }
  }

@mdietze
Copy link
Member

mdietze commented Jun 12, 2020

@mccabete I suspect the more general, scalable solution is to check for the presence of the different output variables themselves, rather than checking which ED2 version you were running, with some priority given to one over the other (i.e. check for variable A, if it's not there check for variable B, if it's not there then skip adding that variable to the nc file). Would also be great to see a PR that includes a fix for everyone, not just your personal branch

@mccabete
Copy link
Contributor Author

mccabete commented Jun 12, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants