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

ECMWF Open Data download and conversion functions #2975

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

Conversation

swarnaleem
Copy link

@swarnaleem swarnaleem commented Jul 21, 2022

ECMWF Open Data 15-day forecast download and conversion to NetCDF (from Grib2)

Description

The download.ECMWF.R function downloads ECMWF Opendata 15-day perturbed forecast (pf) consisting of 50 ensemble members using ecmwf-opendata python package. Currently, it is downloading 50 ensemble members using download_ecmwf_opendata.py. The parameters being downloaded are as follows:

Parameter Short Name Name
2t 2 metre temperature
tp total precipitation
10u 10 metre U wind component
10v 10 metre V wind component
q Specific humidity
r Relative humidity
sp Surface pressure

After downloading, ecmwf_grib2nc.py function converts the grib2 file consisting of 50 ensemble members into 50 separate netCDF files based on their ensemble numbers.

Motivation and Context

PEcAn currently has 17 meteorological drivers available, some of which are Ameriflux, ERA5, and CMIP5 which are used as input data to various models. ECMWF Open Data provides globally available ensemble forecasts that can be used to drive various models (available in PEcAn).

Creating an ECMWF Open Data pipeline in PEcAn would help in adding various meteorological forecast data (weather data) as input variables to the ecosystem models which would help in running them in the future. In this proposal, the interest is around 15-day ensemble forecast that originates from the day of the forecast and goes 360h into the future.

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 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.

Copy link
Contributor

@istfer istfer left a comment

Choose a reason for hiding this comment

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

Thanks Swarnalee for this WIP PR, I started a review (not completed yet). In addition to those comments, could you also update the documentation for ECMWF? I believe this is the right place to do it

modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved
modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved

all_parameters <- c("2t", "tp", "10u", "10v", "q", "r", "sp")

if (tolower(parameters) == "all") {
Copy link
Contributor

Choose a reason for hiding this comment

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

if this condition is not met there will be no parameter_types, then line 61 will most likely throw a 'parameter_types' not found error, should there be an else to this if?

Copy link
Author

Choose a reason for hiding this comment

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

Added else to show what can be given as an input, here all.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is remains to be the case, please add an else to make sure parameter_types are assigned something in case tolower(parameters) != "all"

Copy link
Author

Choose a reason for hiding this comment

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

If tolower(parameters) != "all" should the parameter_types still be "all"?

modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved
modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved
modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved
modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved
modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved
modules/data.atmosphere/R/download.ECMWF.R Show resolved Hide resolved
*modified datasets acc to CF standard
*updated documentation
*resolved PR comments
@swarnaleem swarnaleem changed the title [WIP] ECMWF Open Data download and conversion functions ECMWF Open Data download and conversion functions Jul 25, 2022
@swarnaleem swarnaleem requested a review from istfer July 25, 2022 15:49
Copy link
Contributor

@istfer istfer left a comment

Choose a reason for hiding this comment

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

I'll continue to review the revised code but in the meantime could you please make sure this PR would make ECMWF pipeline run successfully through the workflow. I still don't see a registration file, which should look something like this.

I also suspect that fchour will never be passed to download.ECMWF by upstream functions, i.e. see here. You can either change that fcn arg to product (logically it's not that far off I'd be fine with it) or you need to add fchour to that convert.input call

Copy link
Contributor

@istfer istfer left a comment

Choose a reason for hiding this comment

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

please fix the issues regarding the files/filenames

}

# Removes any already existing files with base name same as `latest_filedate``
if ((length(list.files(path = outfolder, pattern = as.character(latest_filedate)) ) > 0) == TRUE){
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really want to remove or if the global files are already there can we skip the download?

modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved
modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved
modules/data.atmosphere/R/download.ECMWF.R Outdated Show resolved Hide resolved
@swarnaleem swarnaleem requested a review from istfer September 9, 2022 05:14

##### 3h CF -> 0 - 141h at 3h timestep
cf_3h_out_fname <- paste0(latest_filedate, time, stream, "cf_3h", sep = "_")
cf_3h_nc <- combine_files_cf(fnames_3h, lat_in, lon_in, out_filename= cf_3h_out_fname)
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we agreed to not write these intermediate 3hrly and 6 hrly netcdf files, are these still being written?

script.path = file.path(system.file("ECMWF/download_ecmwf.py", package = "PEcAn.data.atmosphere"))
reticulate::source_python(script.path)

date_latestdata <- ecmwflatest(time, step, stream, type, all_parameters)
Copy link
Contributor

@istfer istfer Sep 15, 2022

Choose a reason for hiding this comment

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

Suggested change
date_latestdata <- ecmwflatest(time, step, stream, type, all_parameters)
date_latestdata <- ecmwflatest(time, step_15day, stream, type, all_parameters)

there is no step variable, was this supposed to be step_15day?


in_fname <- paste(latest_filedate, time, stream, sep = "_")

data_download <- ecmwfdownload(date, time, stream, type, all_parameters, in_fname)
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't thoroughly tested, but this function seems to be downloading the grib files to the working directory whereas it probably should use outfolder

@mdietze
Copy link
Member

mdietze commented Mar 21, 2023

@swarnalee-m we'd love to pull in this code. Any chance you can address the few remaining issues?

@mdietze
Copy link
Member

mdietze commented Apr 26, 2023

@swarnalee-m pinging you again to hopefully finish up this PR

@mdietze
Copy link
Member

mdietze commented Sep 3, 2023

@istfer any chance you can finish up this PR? @swarnaleem hasn't touched it in a year and doesn't seem to be responding to inquiries about addressing the changes you requested.

@istfer
Copy link
Contributor

istfer commented Oct 9, 2023

I can give it a try

Copy link

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

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