ESMValGroup / ESMValTool

ESMValTool: A community diagnostic and performance metrics tool for routine evaluation of Earth system models in CMIP
https://www.esmvaltool.org
Apache License 2.0
223 stars 128 forks source link

CMIP5 HadGEM2-ES & HadGEM2-CC ocean models contains latitude > 90. degrees. #378

Closed ledm closed 6 years ago

ledm commented 6 years ago

The grid for the ocean components of the models HadGEM2-ES & HadGEM2-CC on BADC/Jasmin contains latitude coordinates greater than 90 degrees. This fails ESMValTool's CMOR checks with the error message: esmvaltool.cmor.check.CMORCheckError: There were errors in variable tos: lat: has values > valid_max = 90.0

The solution is to add a model specific module to the cmor/_fixes/CMIP5 directory, ie: esmvaltool/cmor/_fixes/CMIP5/HadGEM2_CC.py

from ..fix import Fix
import numpy as np
class tos(Fix):
    def fix_metadata(self, cube):     
        lat = cube.coord('latitude')
        lat.points = np.clip(lat.points,-90.,90.)
        lat.bounds = np.clip(lat.bounds,-90.,90.)
        return cube

This function uses numpy.clip to reduce the minimum and maximum latitude points and boundaries to +/-90. The full code is available upon request, but I don't have permissions to push to the main repository. (There's probably a way around this - forking, perhaps? - I have some other minor contributions to make as well!)

With this model specific fix_data/fix_metadata (is latitude data or metadata?) preprocessor in place, it is called by adding the following lines to the namelist.yml file in the preprocessor section:

    fix_data: 
     model: HadGEM2-CC
     project: CMIP5
     short_name: tos
     cmor_table: CMIP5

As this fix affects all fields, not just tos (Temperature at the Ocean Surface), it should be possible to replace the short_name with allvars. However, this currently does not work as the allvars short_name is not recognised by the CMOR_Table, and will fail the CMORCheck processor.

bouweandela commented 6 years ago

Yes, please fork and create a pull request to merge into the REFACTORING_backend branch. You should not need add a fix_data item in the preprocessor section, this will be automatically done by the namelist parser (in esmvaltool/_namelist.py).

bouweandela commented 6 years ago

To see what the namelist parser made of your namelist, including the exact arguments passed to preprocessor functions, look for the description of the DiagnosticTask and PreprocessorTask objects under 'Namelist summary' in the debug log file in the output directory in run/main_log_debug.txt.

ledm commented 6 years ago

I've created a fork, pushed my code and made a pull request: https://github.com/ESMValGroup/ESMValTool/pull/385

With regards to using allvars in fix_metadata, but allvars not working with CMOR_Table: is it worth creating a separate issue to address that problem?

valeriupredoi commented 6 years ago

@jvegasbsc you the CMOR main man, man

ledm commented 6 years ago

Hi @valeriupredoi,

@jvegasbsc and I have been discussing the CMOR stuff in issue #387. From what I can tell, the issue is that the CF standard used in iris is not compatible with the CMOR standard used in ESMValTool. Any ideas?

valeriupredoi commented 6 years ago

@ledm can you send me a path to such a file pls (preferably BADC)

ledm commented 6 years ago

An good example of a file would be: /badc/cmip5/data/cmip5/output1/CCCma/CanESM2/historical/mon/ocean/Omon/r1i1p1/latest/thetao/thetao_Omon_CanESM2_historical_r1i1p1_200101-200512.nc

This is 3D temperature for the CanESM2 model. I've been using this file for many of my recent tests.

valeriupredoi commented 6 years ago

cheers! am grabbing it and having a looksee right now

valeriupredoi commented 6 years ago

@ledm @jvegasbsc good news, fellas, depth level selection works seamlessly in ESMValTool with that file that @ledm provided me! I only had to change the CMIP5_Omon CMOR table under the thetao variable: dimensions: longitude latitude depth_coord time

valeriupredoi commented 6 years ago

the thing is in the CMOR table generic_levels: olevel is not defined, but I think in this case, the variable needs a purposely defined depth level just as plev in the atmospheric case plev17 or plev19 for CMIP6

valeriupredoi commented 6 years ago

also note one thing -- the netCDF file was built with CMOR version :cmor_version = "2.5.4" whereas the minimum CMOR version imposed by the ESMValTool table is cmor_version: 2.6 ! minimum version of CMOR that can read this table so this may explain why the table has to be modified to work with this file

valeriupredoi commented 6 years ago

OK fellas, I dug deeper in this crap: to be able to use the generic axis one needs to set:

cube.coord('depth').attributes['generic_level'] = True

but this will fail at saving stage to netCDF format because of netcdf4 > 1.2; now, to install netcdf4=1.2.4, one needs to downgrade hdf5 from 1.10 to 1.8.17 (anything equal or above 1.8.18 will not work); now, to downgrade to hdf5=1.8.17, you need to downgrade netcdf-fortran to God knows what version is stable before 4.4; in other words, too much dependency hassle! How about we gather all these changes to be made to the CMOR CMIP Omon table and create a PR on their GitHub page? Easy-peasy, thos files are 5 years old anyway and this will not change science but update an ancient axis

ledm commented 6 years ago

Sounds like a nightmare!

Also, we'd need to apply these changes needed to the other CMOR tables containing the olevel field. Do we need to look at the cmip6 tables too?

jvegreg commented 6 years ago

I've been reviewing all the code for the generic levels, and probably we were trying to be to clever. We have a check for the rank and the generic level checking was only trying to add another layer of security by checking the axis.

I think we should forgot about this and just ignore the check on the generic levels, also because in fact CMOR does not force them to meet any standard, not even names.

What we may want to do is to force the renaming of the extra generic level to avoid complexity in the diagnostics.

jvegreg commented 6 years ago

Abot CF and CMOR, CMOR is more restrictive than CF. All CMOR files are CF-compliant but this is not true the other way around

mattiarighi commented 6 years ago

I think force renaming of the coordinates to a common one is a good idea, for the reason you mentioned. This should apply also to time, lat and lon (some models are using latitude).

valeriupredoi commented 6 years ago

@ledm there are such metadata dysfunctions in CMIP6 as well (see for example IPSL metadata that has negative longitueds and latitudes with reverse order https://github.com/ESMValGroup/ESMValTool/blob/REFACTORING_CMIP6/esmvaltool/cmor/_fixes/CMIP6/IPSL_CM6A_LR.py) -- it would be great f you could have a look at the same types of data you are looking in CMIP5 but in CMIP6 if they and when they become available

ledm commented 6 years ago

Just writing to confirm that removing the call to report_error in the generic_level check in cmor/check.py does not have an adverse affect on the rest of the ocean evaluation code.

I'm happy with this solution, and if it doesn't cause trouble elsewhere, I'm happy to call this issue closed.

mattiarighi commented 6 years ago

I've tested it with another nml and it does not break anything.