HCLIMcom / HCLIM2CMOR

Climate model output rewriting of HCLIM climate model data for CORDEX. Based on the COSMO-CLM routines.
GNU General Public License v3.0
0 stars 5 forks source link

Variables table updating #33

Closed ljoakim closed 3 months ago

ljoakim commented 3 months ago

Fixes WCRP-CORDEX/cordex-cmip6-cmor-tables#22

I've made a first attempt to update the variables table from the github cmor tables. I took a slightly different approach than I originally planned: instead of generating a new table, the current table is read and updated. This means there should already be a row for each variable, with all required columns filled out, except for the columns that are updated from the cmor tables.

Currently only the following columns are updated:

I've updated the table using the update script, please look at the updated table in the PR.

  1. Are there more columns that should be updated?
  2. Should fx variables have a value for cell_methods? (unsure where to put it in the table)
gnikulin commented 3 months ago
ljoakim commented 3 months ago

Note: I've changed the third column of the cape row from CAPE to cape manually.

ljoakim commented 3 months ago

Unfortunately the updated cell method strings in the cmor tables, when inserted into the variable table in this repo, breaks the logic in cmorlight.py, causing an exception. I'm looking into it.

ljoakim commented 3 months ago

Regarding the cell_method problem, there are three functions dealing with this: proc_seasonal, process_file and process_file_fix. The first one, for seasonal (sem), I don't think I have any data to test with. Will there ever be any seasonal data to cmorize?

gnikulin commented 3 months ago

Seasonal means were only defined in CORDEX-CMIP5 and were useless, no seasonal means in CORDEX-CMIP6. In general, we can delete all parts of the code related to the seasonal mean output.

ljoakim commented 3 months ago
ljoakim commented 3 months ago

...and comment is now also added to the table, and to the variable in the output file.

ljoakim commented 3 months ago

I've removed the draft status, so if you think all is ok (with the comments I've added above), this PR should be ready for merge. I'm running a few additional tests, but if anything pops up, we can handle it in a separate issue.

gnikulin commented 3 months ago

Great !!!! A quick look at the var table shows that all changes/fixes should be fine. We can merge this PR and then fix "residual" problems if any.

ljoakim commented 3 months ago

I found one additional error for day/prhmax, where the cell_methods again caused the error. I think I need to discuss which operation to run for which cases. E.g.:

Probably the last occurrence of time: should be used to find the operation, not the first occurrence which is currently used.

ljoakim commented 3 months ago

Probably the last occurrence of time: should be used to find the operation, not the first occurrence which is currently used.

Correction: studying the code and previous behaviour, I think it is correct to use the first occurence.

So, I believe the code as it is now is working for all variables, except for prhmax due to it's particular cell_method. I suggest we open a separate issue for that, to get this PR merged. Then we can start cmorizing other variables for analysis/validation. Would that be ok?

gnikulin commented 3 months ago

Yes, we'll wait until this issue with monthly prhmaxis clarified, https://github.com/WCRP-CORDEX/discuss/issues/7