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

In recipe_wflow, use daily ERA5 data from the new cmorizer. #1599

Merged
merged 9 commits into from Apr 2, 2020

Conversation

Peter9192
Copy link
Contributor

@Peter9192 Peter9192 commented Mar 24, 2020

This PR changes the PCRglob recipe to take advantage of the new era5 cmorizer (#1432)

There was a bug in the old cmorizer. Some cmorized variables were
daily accumulations, and averaging them correctly now changes the valid
time to 11:30. This breaks the diagnostic, because there we do
some calculations between accumulated variables and non-accumulated variables.
Therefore, the regrid time preprocessor has been added.

There was a bug in the old cmorizer. Some cmorized variables were
daily accumulations, and averaging them correctly now changes the valid
time to 11:30. This breaks the diagnostic, because there we do
some calculations between accumulated variables and non-accumulated variables.
Therefore, the regrid time preprocessor has been added.
@Peter9192
Copy link
Contributor Author

Peter9192 commented Mar 25, 2020

@jeromaerts could you have a look? The valid time of our ERA5 data is either 12:00 (tas) or 11:30 (radiation) UTC. They're combined in the DeBruin computation, so we need them to be at the same time points. Currently, the regrid_time preprocessor changes the time points to midnight (without changing the data). Then you get a 12-hour offset. I see two ways to deal with this:

  • Keep the regrid_time preprocessor and, at the end of the diagnostic, shift everything back half a day. We'll still be calculating PET on variables shifted 30 mins in time, but perhaps that negligible?
  • Change the diagnostic to interpolate all data to the same time points (12:00 UTC). We might have to extrapolate half an hour for the last time point in the dataset in that case. Or add the first day of the next year in the recipe.

@SarahAlidoost @sverhoeven @bouweandela @JaroCamphuijsen @nielsdrost

@jeromaerts
Copy link
Contributor

jeromaerts commented Mar 25, 2020

@Peter9192
In my opinion the calculation of PET variables shifted by 30 mins in time is acceptable/ negligible. I am concerned, however, that these type of fixes are a bit hacky.

Although the difference is negligible, I propose to interpolate all data to the same time points.

@bouweandela
Copy link
Member

bouweandela commented Mar 25, 2020

Keep the regrid_time preprocessor and, at the end of the diagnostic, shift everything back half a day.

@Peter9192 In that case I think it would be more transparent to just shift the times in the diagnostic for those variables that need it at the point where this is needed, the 12 hour shift seems to add needless complication.

I propose to interpolate all data to the same time points.

@jeromaerts Note that the daily data is derived from hourly data, it might make more sense to interpolate the hourly data before computing the mean or even change the temporal statistics function so it does a weighted mean instead of only looking at the time points that are contained within a day: time points from 0-23 hours lead to an average time point of 11:30 for instantaneous variables, while it would probably make more sense to include 0 hours of the next day too in this computation, and then give the two time points at 0 hours half the weight of the other time points. However, this could be a considerable amount of work, so we need to decide whether or not this important.

@SarahAlidoost
Copy link
Contributor

SarahAlidoost commented Mar 26, 2020

@Peter9192 @bouweandela @jeromaerts Half-hour time shift is implemented for tas and psl in marrmot diagnostic, here #1600

@Peter9192 Peter9192 changed the title Use daily ERA5 data from the new cmorizer and add regrid_time WFLOW: Use daily ERA5 data from the new cmorizer and add regrid_time Mar 30, 2020
Peter9192 added 2 commits Mar 30, 2020
Instead of using the regrid_time preprocessor, now we
shift the ERA5 data for instantaneous variables 30 minutes
forward. Regrid_time would shift them to midnight, which is
a much larger offset.
@Peter9192 Peter9192 requested a review from SarahAlidoost Mar 31, 2020
@Peter9192 Peter9192 marked this pull request as ready for review Mar 31, 2020
Copy link
Contributor

@SarahAlidoost SarahAlidoost left a comment

@Peter9192 thanks, looks good. Also, could you please update the documentation of wflow here

esmvaltool/diag_scripts/hydrology/wflow.py Show resolved Hide resolved
Peter9192 and others added 3 commits Apr 2, 2020
Co-Authored-By: SarahAlidoost <55081872+SarahAlidoost@users.noreply.github.com>
@Peter9192
Copy link
Contributor Author

Peter9192 commented Apr 2, 2020

update the documentation

Good one! Done :-)

@SarahAlidoost
Copy link
Contributor

SarahAlidoost commented Apr 2, 2020

update the documentation

Good one! Done :-)

Thanks, and also please address the merge conflict.

Conflicts:
	doc/sphinx/source/recipes/recipe_hydrology.rst
@Peter9192 Peter9192 requested a review from bouweandela Apr 2, 2020
@bouweandela bouweandela requested a review from SarahAlidoost Apr 2, 2020
@bouweandela
Copy link
Member

bouweandela commented Apr 2, 2020

@SarahAlidoost Can you please approve the pull request once you're happy with it?
@Peter9192 Could you please update the pull request title? For clarity, it should mention the name of the recipe that is changed and I think you no longer use the regrid_time preprocessor?

@Peter9192 Peter9192 changed the title WFLOW: Use daily ERA5 data from the new cmorizer and add regrid_time In recipe_wflow, use daily ERA5 data from the new cmorizer. Apr 2, 2020
@Peter9192
Copy link
Contributor Author

Peter9192 commented Apr 2, 2020

Could you please update the title?

Done

@SarahAlidoost
Copy link
Contributor

SarahAlidoost commented Apr 2, 2020

@SarahAlidoost Can you please approve the pull request once you're happy with it?
@Peter9192 Could you please update the pull request title? For clarity, it should mention the name of the recipe that is changed and I think you no longer use the regrid_time preprocessor?

@bouweandela Done.

@bouweandela bouweandela merged commit 49f80e4 into master Apr 2, 2020
4 checks passed
@bouweandela bouweandela deleted the update_wflow_recipe branch Apr 2, 2020
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.

None yet

4 participants