Conversation
LisaBock
left a comment
LisaBock
left a comment
There was a problem hiding this comment.
Thanks @axel-lauer !
I started the review and run in two issues:
- there were small modifications in the downloader necessary (see comment there)
- when starting the formatter I got the following error message:
PermissionError: [Errno 13] Permission denied: '/work/bd0854/b380103/esacci-permafrost-weights'
Please update also the branch with the main branch.
| Overwrite already downloaded files | ||
| """ | ||
| if start_date is None: | ||
| start_date = datetime(1997, 1, 1, tzinfo=datetime.timezone.utc) |
There was a problem hiding this comment.
This gives me the following error message:
AttributeError: type object 'datetime.datetime' has no attribute 'timezone'. Did you mean: 'astimezone'?
It worked when I added from datetime import timezone and replaced datetime.timezone with timezone .
There was a problem hiding this comment.
I just removed the time zone stuff. It seems this is not needed (any more): e74218d
I also updated with the latest main resolving the conflict in input.rst.
| mip: Lyr | ||
| raw: ALT | ||
| file: 'ESACCI-PERMAFROST-L4-ALT-*-AREA4_PP-{year}-fv03.0.nc' | ||
| weights_dir: '/work/bd0854/b380103/esacci-permafrost-weights' |
There was a problem hiding this comment.
Hi @axel-lauer ,
if these weights are needed, could you store them in a public/group folder or in the ESMValTool repository?
There was a problem hiding this comment.
The weights file should be generated automatically and is then reused to speed up the regridding. Could you try setting
weights_dir: './esacci-permafrost-weights' in the config file? Not sure which directory to give so it works for every user on every machine.
LisaBock
left a comment
LisaBock
left a comment
There was a problem hiding this comment.
Thanks @axel-lauer !
Works fine for me. As soon as the changes in the ESMValCore are merged, this PR is also ready for merging.
|
Hello, this pull request has been marked with the If you won't be able to finish this in time, don't worry - just unassign the milestone |
jlenh
added
in technical review
requires new ESMValCore release
There was a problem hiding this comment.
Looks good overall, just a few comments!
If you could also deal with the Codacy issues! It looks like mostly using the Path library instead of os, and some looping over dictionary keys. Let me know if you need assistance with that 😄 And there is a conflict to solve with the main branch.
The tests are failing because of the dependency on the newest ESMValCore version. I have also bumped the PR to the next version as it seems that the needed PR in ESMValCore will not be merged for v2.14.0.
| # Common global attributes for CMORizer output | ||
| attributes: | ||
| dataset_id: ESACCI-PERMAFROST | ||
| version: 'v3.0' |
There was a problem hiding this comment.
Out of curiosity, looking at the website, the latest version is v5.0, is there a reason why the v3.0 then?
| # description of ESACCI-PERMAFROST v3.0 polar stereographic | ||
| # grid for cdo | ||
| esagrid = ( | ||
| f"gridtype = projection\n" | ||
| f"gridsize = {totalsize}\n" | ||
| f"xsize = {xsize}\n" | ||
| f"ysize = {ysize}\n" | ||
| f"xname = x\n" | ||
| f'xlongname = "x coordinate of projection"\n' | ||
| f'xunits = "m"\n' | ||
| f"yname = y\n" | ||
| f'ylongname = "y coordinate of projection"\n' | ||
| f'yunits = "m"\n' | ||
| f"xfirst = -6111475.22239475\n" | ||
| f"xinc = 926.625433138333\n" | ||
| f"yfirst = 4114895.09469662\n" | ||
| f"yinc = -926.625433138333\n" | ||
| f"grid_mapping = polar_stereographic\n" | ||
| f"grid_mapping_name = polar_stereographic\n" | ||
| f"straight_vertical_longitude_from_pole = 0.\n" | ||
| f"false_easting = 0.\n" | ||
| f"false_northing = 0.\n" | ||
| f"latitude_of_projection_origin = 90.\n" | ||
| f"standard_parallel = 71.\n" | ||
| f"longitude_of_prime_meridian = 0.\n" | ||
| f"semi_major_axis = 6378137.\n" | ||
| f"inverse_flattening = 298.257223563\n" | ||
| f'proj_params = "+proj=stere +lat_0=90 +lat_ts=71 +lon_0=0' | ||
| f' +k=1" +x_0=0 +y_0=0 +datum=WGS84 +units=m +no_defs"\n' | ||
| ) |
There was a problem hiding this comment.
If you have, could you include a reference or a link where to find this information? Or is it part of the downloaded files? That would be helpful for reference either way.
| logger.info( | ||
| "Generating regridding weights. This will take" | ||
| " about 5-10 minutes (or more)...", | ||
| ) |
There was a problem hiding this comment.
It was not that long after all 😄
| in_file = glob.glob(filepattern)[0] | ||
| if not in_file: | ||
| logger.info( | ||
| "%d: no data not found for variable %s", | ||
| loop_date.year, | ||
| short_name, | ||
| ) | ||
| else: | ||
| _extract_variable(in_file, var, cfg, out_dir, loop_date.year) |
There was a problem hiding this comment.
You need to adapt the code here, otherwise you get an error if no file is found with glob.glob(filepattern) and it returns an empty list. You can similarly evaluate it in the if/else statement afterwards regardless.
| in_file = glob.glob(filepattern)[0] | |
| if not in_file: | |
| logger.info( | |
| "%d: no data not found for variable %s", | |
| loop_date.year, | |
| short_name, | |
| ) | |
| else: | |
| _extract_variable(in_file, var, cfg, out_dir, loop_date.year) | |
| in_file = glob.glob(filepattern) | |
| if not in_file: | |
| logger.info( | |
| "%d: no data not found for variable %s", | |
| loop_date.year, | |
| short_name, | |
| ) | |
| else: | |
| _extract_variable(in_file[0], var, cfg, out_dir, loop_date.year) |
There was a problem hiding this comment.
Actually you could already replace here with a call to the Path library to deal with a Codacy error:
in_file = list(Path(in_dir).glob(var["file"].format(year=loop_date.year)))
| coord = cube.coord(axis=axis) | ||
| if axis == "T": | ||
| coord.convert_units("days since 1850-1-1 00:00:00.0") | ||
| coord.standard_name = coord_def.standard_name | ||
| coord.var_name = coord_def.out_name | ||
| coord.long_name = coord_def.long_name | ||
| coord.points = coord.core_points().astype("float64") | ||
| if len(coord.points) > 1: | ||
| if coord.bounds is not None: | ||
| coord.bounds = None | ||
| coord.guess_bounds() |
There was a problem hiding this comment.
I am wondering about the time axis on the output cube:
- I do not see any bounds on the output cube for the time axis? Would it be possible maybe to add time bounds to indicate it covers the whole year?
| ) | ||
|
|
||
| # delete temporary file | ||
| os.remove(esagrid_file) |
There was a problem hiding this comment.
Would it be possible to keep this one also as a file after processing? It would avoid people having to look deep into ESMValTool's code to find the information.
4 participants
Description
This PR adds a downloading and formatting script for the dataset ESACCI-PERMAFROST v3.0. The following three variables are supported:
The CMORizer requires the custom CMOR tables defined in ESMValGroup/ESMValCore#2358.
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.
New or updated data reformatting script