Cmorize wfde5 #1991

mwjury · 2021-01-18T08:45:14Z

I added a cmorizer for WFDE5 (bias-corrected reconstruction of ERA5). Depends on #1990.

☝ Create an issue to discuss what you are going to do

Checklist

It is the responsibility of the author to make sure the PR is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

🛠 PR has a descriptive title for the changelog
🛠 Code follows the style guide
🛠 Circle/CI tests pass
🛠 Codacy code quality checks pass

New or updated data reformatting script:

🛠 Dataset is added to the table in the documentation
🛠 Documentation contains instructions to obtain the data
🛠 Tests for the CMORized data are available
🧪 Numbers/units of the data look physically meaningful
🛠 Data set is added to the OBS data pool @remi-kazeroni

valeriupredoi · 2021-02-04T13:00:33Z

this looks ready, I added the release tag and we need to fix the iris documentation as per #2011

valeriupredoi

cheers @mwjury - I will merge once we merge the fix to the iris documentation from #2012 🍺

bouweandela · 2021-02-09T16:27:05Z

Just applying some new labels to make the status of all open pull requests more clear. Note that the tests on CircleCI and the documentation build must be successful (green) before the label 'approved by technical reviewer' can be applied.

remi-kazeroni

The version of the data mentioned in this PR (v1.0) is now deprecated. @mwjury coud you please update the downloading instrustions, the version number (v1.1) accordlingly and check whether your script runs fine on the newest data? Thanks!

valeriupredoi · 2021-02-17T11:51:23Z

@remi-kazeroni cheers, mate! Next time please use the review changes button and ask for changes so that the official review is logged and the merge can not happen unless both you as reviewer and the PR creator agree on changes made/not needed 👍

valeriupredoi

Technical review passed by me, but changes needed for the sci review from comment

esmvaltool/cmorizers/obs/cmor_config/WFDE5.yml

hb326 · 2021-03-19T10:31:30Z

I think there is a problem with the calculation of the monthly means. I ran the cmorizer and get for the file "OBS_WFDE5_ground_v1.1-CRU+GPCC_Amon_pr_197901-197912.nc" only one time step. I think the monthly means are overwritten when they are stored in the file. Could you check that again?

hb326 · 2021-04-09T11:26:27Z

@mwjury, you want to check the calculations of the monthly means for this pull request? I am happy to do the scientific review then.

mwjury · 2021-04-16T08:41:27Z

@hb326 In the previous version for v1.0 the output was monthly files (one month per file), so I do not understand how you got files for a year.
I have updated to the current version, could you please have a look.

mwjury · 2021-04-16T08:47:44Z

Sorry it took so long. @valeriupredoi this should be ready to go (FLAKE8-check failing due to regex, any ideas).

Updated to current version (v1.1) @remi-kazeroni .
Accounted for snowfall as raised by @lukruh .
Added yearly treatment of files as implemented in cmorize_WFDE5_monthly_fix by @schlunma , removed cmorization of hourly data, but still keeping cmorizing daily data.

hb326

For me the cmorizer looks good now. I think, providing the option to get "pr" out as the sum from rainfall flux and snowfall flux is great! I also get 12 (different) months now in the monthly file, and 365 (different) days for the daily files.

hb326 · 2021-05-05T08:28:10Z

esmvaltool/cmorizers/obs/cmorize_obs_wfde5.py

+import iris
+from cf_units import Unit
+from esmvalcore.preprocessor import daily_statistics, monthly_statistics
+


When I pulled the branch to test it yesterday, this line was only showing up as
from esmvalcore.preprocessor import monthly_statistics

That made the cmorizer crash, of course. I added the daily_statistics manually, and then it ran fine.
This is weird. @valeriupredoi, could you check what you get when you pull the branch?

@valeriupredoi: another question I had last week. :)

gimme a few, entering another meeting in 10min (unplanned meetings blegh)

it's possible you pulled a local copy, or that your git cache may be a bit special 😁 Anyway I pulled the branch from remote and that was there all fine, I now merged master in so all should be tiptop. I'll review it now 👍

zklaus · 2021-05-05T08:44:50Z

(FLAKE8-check failing due to regex, any ideas).

Yes! According to https://www.flake8rules.com/rules/W605.html you should write your regex in a raw string, i.e. r'bla'. The reason is that without the r, Python will replace all occurrences of a backslash followed by something with a control character right then and there, before even passing it to the regex library. So '\n' becomes a newline, '\r' a carriage return, etc. The reason why it still works as it stands is that the backslashes are not followed by any recognized control sequences and therefore are in the end not replaced. This is not completely accidental since back in the days the designers of regular expressions had this in mind, but it might also not be guaranteed, which is why Flake8 is saying it's better to tell Python that you don't want it replacing backslashes here.

valeriupredoi · 2021-05-11T16:36:51Z

(FLAKE8-check failing due to regex, any ideas).

Yes! According to https://www.flake8rules.com/rules/W605.html you should write your regex in a raw string, i.e. r'bla'. The reason is that without the r, Python will replace all occurrences of a backslash followed by something with a control character right then and there, before even passing it to the regex library. So '\n' becomes a newline, '\r' a carriage return, etc. The reason why it still works as it stands is that the backslashes are not followed by any recognized control sequences and therefore are in the end not replaced. This is not completely accidental since back in the days the designers of regular expressions had this in mind, but it might also not be guaranteed, which is why Flake8 is saying it's better to tell Python that you don't want it replacing backslashes here.

# noqa FTW

valeriupredoi

good stuff @mwjury - couple minor comments from me 🍺

esmvaltool/cmorizers/obs/cmorize_obs_wfde5.py

zklaus · 2021-05-11T19:30:56Z

# noqa FTW

No :( In this case, fix the code. This really should be a raw string. Alternatively, escape your backslashes.

Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>

remi-kazeroni · 2021-06-28T09:10:28Z

I have just run this cmorizer to put the cmorized data in the obs folder. I realized there is a typo in the WFDE5.yml config file which should have: tier: 2 and not tier: 3. The cmorizer runs fine but the recipe_check_obs.yml not... Sorry, I should have jumped in here earlier and see this while cmorizing the data before this is merged. But in such cases @valeriupredoi please ping me before merging and I'll do my best to have a look asap 👍 The last box was not checked and we agreed that this is my task 😄 Shall I restore the branch or fix this issue in another PR?

valeriupredoi · 2021-06-28T09:41:56Z

ouch, totally my bad, sorry - I did notice the last box being unchecked but I thought someone forgot to check it 😁 No need to restore the branch, just do a quick PR with the typo correction and I'll approve it pronto 👍

mwjury added 5 commits January 18, 2021 09:26

initial commit

df7c04b

add test

4486f51

add description

ba39b55

further description update

13d5e5b

remove redundancy

1779594

bouweandela added the observations label Jan 25, 2021

Merge branch 'master' into cmorize_WFDE5

ea3a35f

valeriupredoi added the release label Feb 4, 2021

valeriupredoi approved these changes Feb 4, 2021

View reviewed changes

mwjury mentioned this pull request Feb 4, 2021

Do not assume in save_variable that a cube with only one timestep is yearly data #1990

Merged

5 tasks

remi-kazeroni self-requested a review February 4, 2021 16:08

bouweandela added approved by technical reviewer looking for scientific reviewer in technical review and removed approved by technical reviewer labels Feb 9, 2021

remi-kazeroni mentioned this pull request Feb 9, 2021

Cmorize gpcc fix #1982

Merged

9 tasks

remi-kazeroni reviewed Feb 10, 2021

View reviewed changes

Merge branch 'master' into cmorize_WFDE5

bb90ffb

valeriupredoi requested changes Feb 17, 2021

View reviewed changes

valeriupredoi added approved by technical reviewer in scientific review and removed in technical review looking for scientific reviewer labels Feb 17, 2021

lukruh reviewed Mar 4, 2021

View reviewed changes

esmvaltool/cmorizers/obs/cmor_config/WFDE5.yml Show resolved Hide resolved

Merge branch 'master' into cmorize_WFDE5

c8126ff

mwjury added 2 commits April 16, 2021 10:34

update to v1.1, remove hourly output

f17931a

style checks

9b25520

update recipe checks

e0dde08

hb326 reviewed May 5, 2021

View reviewed changes

valeriupredoi added 2 commits May 11, 2021 17:14

Merge branch 'master' into cmorize_WFDE5

98a5d48

shut pylint up, that is a regexp not raw string

5a494e2

valeriupredoi requested changes May 11, 2021

View reviewed changes

esmvaltool/cmorizers/obs/cmorize_obs_wfde5.py Show resolved Hide resolved

esmvaltool/cmorizers/obs/cmorize_obs_wfde5.py Show resolved Hide resolved

esmvaltool/cmorizers/obs/cmorize_obs_wfde5.py Outdated Show resolved Hide resolved

mwjury and others added 2 commits May 18, 2021 10:34

Update esmvaltool/cmorizers/obs/cmorize_obs_wfde5.py

f4eaca9

Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>

fix regex to raw string

967fd44

mwjury requested a review from valeriupredoi June 1, 2021 08:06

valeriupredoi approved these changes Jun 24, 2021

View reviewed changes

Merge branch 'main' into cmorize_WFDE5

383e2a2

valeriupredoi merged commit 39764f4 into main Jun 24, 2021

valeriupredoi deleted the cmorize_WFDE5 branch June 24, 2021 16:02

This was referenced Jun 28, 2021

Fix the WFDE5 cmorizer #2211

Merged

Missing data for the WFDE5 cmorizer #2215

Closed

remi-kazeroni mentioned this pull request Sep 17, 2021

Streamline observations download #1657

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cmorize wfde5 #1991

Cmorize wfde5 #1991

mwjury commented Jan 18, 2021 •

edited by valeriupredoi

Loading

valeriupredoi commented Feb 4, 2021

valeriupredoi left a comment

bouweandela commented Feb 9, 2021

remi-kazeroni left a comment •

edited

Loading

valeriupredoi commented Feb 17, 2021

valeriupredoi left a comment

hb326 commented Mar 19, 2021

hb326 commented Apr 9, 2021

mwjury commented Apr 16, 2021

mwjury commented Apr 16, 2021

hb326 left a comment

hb326 May 5, 2021

hb326 May 11, 2021

valeriupredoi May 11, 2021

valeriupredoi May 11, 2021

zklaus commented May 5, 2021

valeriupredoi commented May 11, 2021

valeriupredoi left a comment •

edited

Loading

zklaus commented May 11, 2021

remi-kazeroni commented Jun 28, 2021

valeriupredoi commented Jun 28, 2021

Cmorize wfde5 #1991

Cmorize wfde5 #1991

Conversation

mwjury commented Jan 18, 2021 • edited by valeriupredoi Loading

Checklist

New or updated data reformatting script:

valeriupredoi commented Feb 4, 2021

valeriupredoi left a comment

Choose a reason for hiding this comment

bouweandela commented Feb 9, 2021

remi-kazeroni left a comment • edited Loading

Choose a reason for hiding this comment

valeriupredoi commented Feb 17, 2021

valeriupredoi left a comment

Choose a reason for hiding this comment

hb326 commented Mar 19, 2021

hb326 commented Apr 9, 2021

mwjury commented Apr 16, 2021

mwjury commented Apr 16, 2021

hb326 left a comment

Choose a reason for hiding this comment

hb326 May 5, 2021

Choose a reason for hiding this comment

hb326 May 11, 2021

Choose a reason for hiding this comment

valeriupredoi May 11, 2021

Choose a reason for hiding this comment

valeriupredoi May 11, 2021

Choose a reason for hiding this comment

zklaus commented May 5, 2021

valeriupredoi commented May 11, 2021

valeriupredoi left a comment • edited Loading

Choose a reason for hiding this comment

zklaus commented May 11, 2021

remi-kazeroni commented Jun 28, 2021

valeriupredoi commented Jun 28, 2021

mwjury commented Jan 18, 2021 •

edited by valeriupredoi

Loading

remi-kazeroni left a comment •

edited

Loading

valeriupredoi left a comment •

edited

Loading