met2model.ED2 skips over leap years

[mccabete]

Only confirmed this error for GFDL, but might happen for others. met2model.ED2 flags all leap years as incomplete and warns "____ is not a complete year and will not be included" and skips them. This makes ED2 fail for runs that span leap years.

Example error run here.

[mccabete]

Workaround
GFDL has no leap-years built into it's met. My personal workaround is to check if the met is GFDL and if it is, treat all leap years like non-leap years. This could use a more general check for met products that don't have leap-years.

[mdietze]

@mccabe, that work around won't work -- it will get you through the PEcAn met workflow but models that need leap years will still be one day short, and thus will crash. I think you really do need to copy and add in an extra day

[mccabete]

I agree. My solution is completely hackey, and I intend to sequester it in a personal branch for the time being. ED seems fine with it for now, so I think I'll return to this problem after ESA and implement a real solution.

[istfer]

I think this is not a met2model.ED2 bug, but it is due to the GFDL not being processed properly (assuming GFDL has leap years), there is no check for leap years in the download.GFDL code, it's always 2920 values per year

Also met2model.ED processes leap years for AmerifluxLBL

[mdietze]

I think we agreed previously that the issue was with the GFDL met, the question is whether the solution should come within the GFDL download, within met2model, or be something generic in between. Either way, that extra leap day needs to be added for models that need it.

[istfer]

got it, I wasn't sure whether GFDL has leap years or not (should've read the thread more carefully, not just the title :))

[ankurdesai]

Many modeled met products (and even some obs) skip leap years, some older climate models even used a 360 day year. So we either need to decide that all met products must gap-fill leap days (for example, by replicating Feb 28 twice) if not provided, or all met2model must do that step if the model requires it. Since this seems model specific (some models don't care), seems like met2model.

[github-actions]

This issue is stale because it has been open 365 days with no activity.

[ashiklom]

I think this has been addressed. That function takes a leap_year argument that controls this behavior. I'm not sure how that plays with GFDL met, though.

pecan/models/ed/R/met2model.ED2.R

Lines 26 to 28 in 9ed21c9

	#' @param leap_year Enforce Leap-years? If set to TRUE, will require leap years to have 366 days. If set to false, will require all years to have 365 days. Default = TRUE.
	met2model.ED2 <- function(in.path, in.prefix, outfolder, start_date, end_date, lst = 0, lat = NA,
	lon = NA, overwrite = FALSE, verbose = FALSE, leap_year = TRUE, ...) {