-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implement ERA5 data ingestion #11
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To review this PR, the reviewer can simply test the functionality of the code using the demo notebook era5_dataset_demo.ipynb
.
Also, it is noteworthy to mention two things:
- The implementation only concerns the variables needed for running stemmus-scope model (got from https://github.com/EcoExtreML/STEMMUS_SCOPE_Processing/blob/main/global_data/downloading_global_data.md). Given the wide range of fields available in ERA5 and our future plan to make the tool more generic, more variables need to be added to the reference variable list later.
- A missing license agreement will trigger an error during downloading (see the example below). Since via
cdsapi
the error message is very clear, it is not necessary to put an extra layer (error message catcher) inzampy
to handle this.
e.g. for downloading the land-cover data from cds without accepting the agreement:
Exception: Client has not agreed to the required terms and conditions. To access this resource, you first need to accept the termsof 'ESA CCI licence' at https://cds.climate.copernicus.eu/cdsapp/#!/terms/satellite-land-cover.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Yang, nice work! It works well for most variables, and the code and tests look quite clean.
I tried downloading the "surface_thermal_radiation" variable, but the conversion is broken. The name is not consistent in the ALMA convention definition "surface_thermal_radiation" vs "net_longwave_radiation".
Additionally, the ERA5 values are (frustratingly) accumulated values. In PyStemmusScope we use the ERA5-land variables which are not accumulated, but fluxes.
I think deaccumulation might be simplest using np.diff
, and a xr.where(ds.time.dt.hour==0, ...)
operation to get the 00:00 data correct. This can be done during ingestion.
Thanks for your review @BSchilperoort !
I think "surface_thermal_radiation" vs "net_longwave_radiation" are two different things ("net_longwave_radiation" is the net radiation between downward and upward ones, while "surface_thermal_radiation" is the downward one only). But I have added the corresponding "surface_thermal_radiation" and "surface_solar_radiation" variables to the ALMA convention list. Thanks for spotting this.
Follow our discussion, these surface radiation variables are instantaneous (😌 luckily), therefore we don't need to be bothered by the "decumulation“. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Yang, just some relatively small comments from me :)
If you have any questions just let me know.
Co-authored-by: Bart Schilperoort <[email protected]>
Kudos, SonarCloud Quality Gate passed! |
This PR adds ERA5 to the dataset collection:
This PR also closes #12.