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

ERA5 on-the-fly CMORizer: changed sign of evspsbl and evspsblpot #2115

Merged
merged 7 commits into from Aug 10, 2023

Conversation

katjaweigel
Copy link
Contributor

@katjaweigel katjaweigel commented Jun 27, 2023

Change sign of evspsbl and evspsblpot to have the same direction as CMIP models. Added both variables for ERA5_land.

Description

For ERA5 evapotranspiration and potential evapotranspiration are negative, if they go from the surface into the atmosphere for CMIP models they are positive:

According to the description of Evapotranspiration on the Copernicus page (https://cds.climate.copernicus.eu/cdsapp#!/dataset/reanalysis-era5-single-levels-monthly-means?tab=overview):

The ECMWF Integrated Forecasting System (IFS) convention is that downward fluxes are positive. Therefore, negative values indicate evaporation and positive values indicate condensation.

In the CMOR table, these fluxes are defined as positive, if they go from the surface into the atmosphere:

Evaporation at surface (also known as evapotranspiration): flux of water into the atmosphere due to conversion of both liquid and solid phases to vapor (from underlying surface and vegetation)

Therefore the fix to cmorize the ERA5 (and ERA5-Land) data needs to switch their sign.

Note that the sign of these variables would change, everywhere where they are used. So far, I did not find any plot or calculation based on these variables from ERA5, but they are included in
ESMValTool/esmvaltool/recipes/cmorizers/recipe_daily_era5.yml
and
ESMValTool/esmvaltool/recipes/examples/recipe_check_obs.yml
and could have been used to create data to use outside ESMValTool diagnostics.

Closes #2116

Link to documentation: https://github.com/ESMValGroup/ESMValCore/blob/fix_era5_evspsbl/doc/quickstart/find_data.rst


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the πŸ›  Technical or πŸ§ͺ Scientific review.


To help with the number pull requests:

…MIP models. Added both variables for ERA5_land.
@katjaweigel
Copy link
Contributor Author

This is not ready, but I accidentally pressed the wrong button, when I wanted to make a draft PR.

@katjaweigel
Copy link
Contributor Author

It seems to fail on hourly data - I don't understand if there is really an issue with them (could be, because there is a time dependent unit conversion involved, but that has actually not changed) or if it does not have the hourly data (we don't have them on Levante but they exist on the Copernicus page)?

@katjaweigel katjaweigel marked this pull request as draft June 27, 2023 15:24
@rswamina
Copy link

@valeriupredoi - Calling your attention to this as requested in the morning session of the workshop.

@valeriupredoi
Copy link
Contributor

@katjaweigel it was just a test needing fixing for the two evspbl hourly variables - if the expected data before fixing is negative then the test data has to be negative as well 😁

@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Merging #2115 (b9158c8) into main (f5aef4a) will increase coverage by 0.00%.
Report is 25 commits behind head on main.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2115   +/-   ##
=======================================
  Coverage   93.09%   93.09%           
=======================================
  Files         237      237           
  Lines       12806    12810    +4     
=======================================
+ Hits        11922    11926    +4     
  Misses        884      884           
Files Changed Coverage Ξ”
esmvalcore/cmor/_fixes/native6/era5.py 91.20% <100.00%> (+0.07%) ⬆️
esmvalcore/cmor/_fixes/native6/era5_land.py 100.00% <100.00%> (ΓΈ)

@katjaweigel
Copy link
Contributor Author

@valeriupredoi thanks a lot, I'd have never found that test.

@valeriupredoi
Copy link
Contributor

@valeriupredoi thanks a lot, I'd have never found that test.

my pleasure, am the chief mechanic after all 😁

@katjaweigel katjaweigel added fix for dataset Related to dataset-specific fix files cmor Related to the CMOR standard backwards incompatible change labels Jun 29, 2023
@katjaweigel katjaweigel self-assigned this Jun 29, 2023
@katjaweigel katjaweigel marked this pull request as ready for review June 29, 2023 14:58
Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

many thanks @katjaweigel 🍺

@valeriupredoi valeriupredoi added this to the v2.10.0 milestone Jul 3, 2023
@remi-kazeroni remi-kazeroni changed the title Change sign of evspsbl and evspsblpot ERA5 on-the-fly CMORizer: changed sign of evspsbl and evspsblpot Jul 20, 2023
@remi-kazeroni remi-kazeroni added bug Something isn't working and removed backwards incompatible change bug Something isn't working labels Jul 20, 2023
Copy link
Contributor

@remi-kazeroni remi-kazeroni left a comment

Choose a reason for hiding this comment

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

Thanks, @katjaweigel! Looks good to me. I just have a couple of minor suggestions for the documentation. I have edited the title of the PR to mention ERA5, feel free to revert back of you prefer.

I have also removed the "backward incompatible" label because this PR corrects a bug in a CMORization for variables not following CMOR standards. I think that label is not really needed here. Question: do we have recipes in main that would be affected by this change? If so, could you please list them in the description of this PR?

esmvalcore/cmor/_fixes/native6/era5.py Show resolved Hide resolved
doc/quickstart/find_data.rst Outdated Show resolved Hide resolved
katjaweigel and others added 2 commits July 24, 2023 14:22
Co-authored-by: RΓ©mi Kazeroni <remi.kazeroni@dlr.de>
@katjaweigel
Copy link
Contributor Author

Dear @remi-kazeroni thanks for checking! I included your comments. the only recipes in main, which contain these variables from ERA5 I found are:
ESMValTool/esmvaltool/recipes/examples/recipe_check_obs.yml and
ESMValTool/esmvaltool/recipes/cmorizers/recipe_daily_era5.yml
I included that in the description above.

Copy link
Contributor

@axel-lauer axel-lauer left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks fot spotting and fixing this! Maybe consider changing the wording in line 132 of find_data.rst e.g. as suggested.

doc/quickstart/find_data.rst Outdated Show resolved Hide resolved
Co-authored-by: Axel Lauer <axel.lauer@dlr.de>
@katjaweigel
Copy link
Contributor Author

@axel-lauer thanks a lot and sorry that I forgot that change when I included the other ones.

@remi-kazeroni remi-kazeroni merged commit ca5e001 into main Aug 10, 2023
4 checks passed
@remi-kazeroni remi-kazeroni deleted the fix_era5_evspsbl branch August 10, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmor Related to the CMOR standard fix for dataset Related to dataset-specific fix files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dataset problem: In ERA5 evspsbl and evspsblpot have the opposite sign compared to CMOR
5 participants