Skip to content

added the viirs LAI download and mosaicing features into pecan. - #4211

Merged
infotroph merged 7 commits into
PecanProject:developfrom
DongchenZ:viirs_lai
Nov 7, 2026
Merged

infotroph merged 7 commits into
PecanProject:developfrom
DongchenZ:viirs_lai

Conversation

@DongchenZ

@DongchenZ DongchenZ commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Added the VIIRS LAI download and mosaicing feature into pecan. Since the MODIS LAI product will soon be decommissioned, it's important to bring another global LAI product, the global VIIRS LAI product, on board. This PR ensures smooth downloading and post-processing of VIIRS H5 files. As before, the user can specify the data DOI and spatiotemporal range to download the data and combine all tiles.

Description

Motivation and Context

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

#' @author Dongchen Zhang
#' @importFrom purrr %>%
merge_image_tiles <- function(in.path,
doi = doi,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is a new function argument, it makes sense for this to have a default, but it's unclear what this specific default is pointing to. Would a NULL be a better default?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I added the NULL default throughout the script.

#' target_format = ".tif")
#' }
gdal_conversion <- function(in_path, outfolder = NULL, band_name = NULL, tile_id = NULL, just_band_name = TRUE, target_format = ".tif") {
gdal_conversion <- function(in_path, doi, outfolder = NULL, band_name = NULL, tile_id = NULL, just_band_name = TRUE, target_format = ".tif") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like new doi argument should have a default

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@DongchenZ

Copy link
Copy Markdown
Contributor Author

@ashiklom Hi Alexy, for this PR, do you have a better way to get the projection info from the VIIRS LAI image than manually calculating it from the tile ID?

@ashiklom

ashiklom commented Nov 4, 2026

Copy link
Copy Markdown
Member

@ashiklom Hi Alexy, for this PR, do you have a better way to get the projection info from the VIIRS LAI image than manually calculating it from the tile ID?

I would expect this to be somewhere in the HDF5 attributes. Poke around the files with an HDF5 package (I like h5py or h5netcdf +Xarray in Python; ncdf4 or RNetCDF may work too and see if you find something that looks like an extent. GDAL might also hoist attributes into raster metadata somehow, so the terra object you create might have some attached attributes you can use for this.

That said, your math is cheap and accurate so I think this is fine.

granules_href <- granules_href[-inds]
# This filter will only apply to the GEDI L4A product.
# Please let the author know if you find any exceptions.
if (doi == "10.3334/ORNLDAAC/2056") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would strongly advise against using DOI for this. In practice, nobody is going to remember the random digits at the front, so they will just have a lookup table like "VIIRS_LAI = ..., GEDI_AGB = ...". I would just key on human readable names like these directly, and error/warn quickly and loudly on any products passed that are not in that list.

If you must key on an unambiguous identifier, I would use the CMR collection ID, since that at least can be used for other CMR queries and is not dependent on exactly how DOIs get resolved (which I think is typically to product pages, not necessarily to specific collections).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I added the doi_to_product_name function to make it easier to map between DOI and product name. Using this function, I changed this line from a DOI match to an actual product name match, so it's easier for the user to understand what's happening here.

doi_to_product_name <- function(doi = NULL) {
# if we doi is NULL.
if (is.null(doi)) {
return(doi)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that all your uses of this function are in if statements (e.g., doi_to_product_name(doi) == "GEDI_L4A") and that comparisons like NULL == "GEDI_L4A" don't return FALSE they return logical(0). I'd propose that a NULL doi return a blank name ""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@mdietze
mdietze added this pull request to the merge queue Nov 6, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 6, 2026
@infotroph
infotroph added this pull request to the merge queue Nov 7, 2026
Merged via the queue into PecanProject:develop with commit 9fa7807 Nov 7, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants