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

Create CalAdapt Download Functions for LOCA downscaled CMIP-5 #3446

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

dlebauer
Copy link
Member

@dlebauer dlebauer commented Feb 19, 2025

Description

Adds download_caladapt_loca to download Cal-Adapt LOCA downscaled CMIP5 rasters inputs that can subsequently be used for data extraction. Accepts sf, sfc, and SpatVector objects.

Motivation and Context

To support CCMMF project #3445

support for WRF-downscaled CMIP6 data will be added in a separate PR

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.

@dlebauer dlebauer added the ccmmf issues and pre related to the ccmmf project label Feb 19, 2025
@dlebauer dlebauer self-assigned this Feb 19, 2025
@dlebauer dlebauer changed the title Create CalAdapt Download Functions Create CalAdapt Download Functions for LOCA downscaled CMIP-5 Feb 19, 2025
@@ -24,6 +24,7 @@ Imports:
amerifluxr,
assertthat,
arrow,
caladaptr,
Copy link
Member

Choose a reason for hiding this comment

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

Recommend making caladaptr a "suggests" since it's only used by one function outside of the core workflow

#' out_dir = "data"
#' )
#' }
download_caladapt_loca_raster <- function(sf_obj,
Copy link
Member

Choose a reason for hiding this comment

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

I get the reason for accepting a sf_obj instead of just lat/lon, which is non-standard for PEcAn met files, but beyond that I'd highly recommend doing everything else you an to adhere to workflow norms and patterns. These include naming the function download. and having the required inputs start with (outfolder, start_date, end_date), followed by spatial information, followed by product-specific variables. This order is also consistent with R norms of having variables that have defaults AFTER those that don't have a default (start/end). To that end, I'd recommend dropping the default on outfolder (what you call outdir) and renaming start_year and end_year to start_date and end_date

period = period,
start_year = start_year,
end_year = end_year,
raster = rast_file
Copy link
Member

Choose a reason for hiding this comment

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

Are you planning on writing separate functions convert the raster to netCDF CF and extract info from it?

Copy link
Member Author

@dlebauer dlebauer Feb 20, 2025

Choose a reason for hiding this comment

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

yep download --> met2cf --> met2model

but next step is to write the download functions for the WRF downscaled CMIP-6. Hopefully (🤞🏻) met2cf.caladapt will work on both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ccmmf issues and pre related to the ccmmf project Dockerfile Documentation Modules Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants