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

updates for mip-cmor-tables #709

Closed
durack1 opened this issue Sep 24, 2023 · 4 comments · Fixed by #722
Closed

updates for mip-cmor-tables #709

durack1 opened this issue Sep 24, 2023 · 4 comments · Fixed by #722

Comments

@durack1
Copy link
Contributor

durack1 commented Sep 24, 2023

From: Matthew Mizielinski
Date: Friday, September 22, 2023 at 5:55 AM
To: Daniel Ellis
Cc: "Durack, Paul J."
Subject: CMOR testing with the mip-cmor-tables

Dan, cc: Paul,

I mentioned earlier that I had been tinkering with directly using the MIP cmor tables as they are now with CMOR (via our tool MIP Convert).

I’ve managed to get this working with the following tinkering

• dimensions changed from a list of strings to a space separated string
• changing the cmor_version in the header to 3.7.2

There are a lot of warnings due to all the additional information in the tables (e.g. provenance, branded variable name), but it worked.

I’m tempted to request a bugfix to CMOR now, i.e. 3.7.3, to allow the dimensions entry to be a string or a list. At which point the mip-cmor-tables are usable as they stand.

Would you be able to raise an issue on CMOR github to request this functionality (I’m sure Paul would be happy to support J)?

Matt

@ping mauzey1 @matthew-mizielinski @wolfiex - Chris this is that problem I had mentioned in #708

@mauzey1 this is great, thanks!

There maybe some very subtle changes required to deal with the new MIP-cmor-tables, a string -> list type change for coord descriptions, so then we can wrap that up as 3.7.3 or 3.8 if the osx-arm64 change happens in the same release

Ping @matthew-mizielinski @taylor13

@durack1
Copy link
Contributor Author

durack1 commented Oct 3, 2023

Just linking across repos so we don't miss fixes
cmip-ipo-internal/CVTool#16

@durack1
Copy link
Contributor Author

durack1 commented Nov 27, 2023

This is a dupe of #718 - so both should be closed when the 3.8.0 changes are in place

@mauzey1
Copy link
Collaborator

mauzey1 commented Jan 12, 2024

As mentioned in the mip-cmor-tables wiki, the modeling_realm attribute in variable entries will become a list instead of a string. We want to concatenate the values in that list into a string of values separated by spaces.

["ocean", "land"] -> "ocean land"

The changes that I made to turn the list of dimensions into a space-separated list string also applies this change to the modeling realm values. However, when I tried creating a NetCDF file where the modeling_realm attribute had multiple values I only got the first value saved to the NetCDF file. This is being caused by the following code.

cmor/Src/cmor.c

Lines 2941 to 2952 in 047fd2c

if (cmor_tables[nVarRefTblID].vars[ref_var_id].realm[0] != '\0') {
szToken = strtok(cmor_tables[nVarRefTblID].vars[ref_var_id].realm, " ");
if (szToken != NULL) {
cmor_set_cur_dataset_attribute_internal(GLOBAL_ATT_REALM,
szToken, 0);
} else {
cmor_set_cur_dataset_attribute_internal(GLOBAL_ATT_REALM,
cmor_tables
[nVarRefTblID].vars
[ref_var_id].realm, 0);
}
} else {

I'm not sure why it was decided to only use the first value in a list of realms. Should this be removed so that every value in the list is included?

@durack1 @matthew-mizielinski

@durack1
Copy link
Contributor Author

durack1 commented Jan 12, 2024

@mauzey1 this seems like another bug. The realm is not being used in either the DRS/directory or filename (the table_id captures this info and is included in both) so should be written out to the file as provided by the table entries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants