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

ERA-Interim Land #1338

Merged
merged 32 commits into from
Jan 13, 2020
Merged

ERA-Interim Land #1338

merged 32 commits into from
Jan 13, 2020

Conversation

bascrezee
Copy link
Contributor

Another addition to the ERA family. Note that the code is very similar to ERA5. A combination of the ERA family to avoid code duplication is under discussion in #1314 and #1312. I needed this functionality right now, so I added it. Depending on the outcome of our discussion we might want to combine several PRs into a single one for the ERA family of datasets.

@bouweandela
Copy link
Member

Hi Bas,

Maybe you could have a look at the ERA-Interim cmorizer and re-use that?

I think you could add your variable to the download script here: https://github.com/ESMValGroup/ESMValTool/blob/version2_development/esmvaltool/cmorizers/obs/download_scripts/download_era_interim.py, I think you could just just add a function that looks something like

_get_land_data(LAND_PARAMS, LAND_TIMESTEPS, years, server, era_interim_land_dir)

or something.

Ideally you could maybe simply re-use the complete cmorizer script by making the content of cmorize_obs_era_interim_land.py just from .cmorize_obs_era_interim import cmorization.

Do you think that could work?

I would also suggest the name ERA-Interim-Land for the dataset, that makes it consistent with the naming for ERA5 and ERA5-Land in the CDS.

@bascrezee
Copy link
Contributor Author

I added the download option in #1423 and will implement the other changes in this PR.

@bascrezee
Copy link
Contributor Author

Your suggested approach seems to work! pylint doesn't like it, but I guess we don't care about that in this very particular case.

I do still get an error though, but the cause might be somewhere else:

2019-11-04 13:46:41,863 [29534] INFO     esmvaltool.cmorizers.obs.cmorize_obs_era_interim,297	CMORizing variable 'sm' from input files '/PROJECTS/C3S/datadir/rawobsdir/Tier3/ERA-Interim-Land/eraintland_swvl1_2010.nc'
concurrent.futures.process._RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/conda/envs/esmvaltool-public/lib/python3.7/concurrent/futures/process.py", line 232, in _process_worker
    r = call_item.fn(*call_item.args, **call_item.kwargs)
  File "/home/ESMValTool/esmvaltool/cmorizers/obs/cmorize_obs_era_interim.py", line 309, in _extract_variable
    cube.standard_name = definition.standard_name
  File "/conda/envs/esmvaltool-public/lib/python3.7/site-packages/iris/_cube_coord_common.py", line 128, in standard_name
    raise ValueError('%r is not a valid standard_name' % name)
ValueError: '' is not a valid standard_name

@bouweandela
Copy link
Member

I fixed the two issues mentioned above.

@bascrezee bascrezee marked this pull request as ready for review January 10, 2020 08:36
@bascrezee
Copy link
Contributor Author

This PR extends the current cmorizer for ERA-Interim to also include ERA-Interim-Land.

@mattiarighi
Copy link
Contributor

@bouweandela can I test it or do you have further comments?

@bascrezee

This comment has been minimized.

@mattiarighi

This comment has been minimized.

@bascrezee

This comment has been minimized.

@mattiarighi
Copy link
Contributor

Just for updating our DKRZ/Jasmin pool, which time frame do you plan to use for this dataset?

@bascrezee
Copy link
Contributor Author

Just for updating our DKRZ/Jasmin pool, which time frame do you plan to use for this dataset?

1988 to 2010

@mattiarighi mattiarighi merged commit 35e0c2b into master Jan 13, 2020
@mattiarighi mattiarighi deleted the ei-land branch January 13, 2020 16:25
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.

3 participants