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

PrePARE cell_measures changed in 3.4.0 #439

Closed
jonseddon opened this issue Feb 12, 2019 · 11 comments · Fixed by #483
Closed

PrePARE cell_measures changed in 3.4.0 #439

jonseddon opened this issue Feb 12, 2019 · 11 comments · Fixed by #483
Assignees
Milestone

Comments

@jonseddon
Copy link
Contributor

I have a file with "cell_measures": "area: areacello". The MIP table for this variable (Omon.ficeberg) has "cell_measures": "area: areacello volume: volcello".

PrePARE from CMOR 3.3.2 failed this file. PrePARE from CMOR 3.4.0 is passing this file. Both versions were installed from Conda. Which behaviour is correct? We were expecting a failure.

I don't know if I'm reading it incorrectly, but from https://github.com/PCMDI/cmor/blame/master/LibCV/PrePARE/PrePARE.py#L586 I think that the regexp only returns a value if the cell_measures in the table contains "OR" (for example "area: areacello OR areacella" in SImon.sisnmass in 01.00.23 of the tables). I think that if there isn't an OR then values will be an empty list and the rest of the checks for cell_measures will pass.

The netCDF file that I've been checking is available at https://drive.google.com/file/d/1_O8JukXI6bhdIhNRPyxvWsroWsFsyQqb/view?usp=sharing

Thanks,

Jon

@taylor13
Copy link
Collaborator

@jonseddon @martinjuckes: Do you agree that ideally PrePARE should do the following:

  1. If table value of areacella is "-OPT", "-UGRID", or "-MODEL", or "", the cell_measures attribute should be omitted. If "cell_measures" is found in the file, then PrePARE should throw an error.
  2. In early versions of CMOR tables, cell_measures could have a value "area: areacello OR areacella". CMOR should skip checking the cell_measures in this case (i.e., if in the table cell_measures contains the string " OR ", do not check cell_measures.
  3. For all other cases,
  • check that table value = file value of cell_measures; throw an error if not the same.
  • check that "external_variables" contains the variables in cell_measures. (e.g., if cell_measures = "area: areacello volume:volcello", then external_variables should contain both "areacello" and "volcello".

@mauzey1
Copy link
Collaborator

mauzey1 commented Feb 12, 2019

The reason why the file didn't pass using CMOR 3.3.2's PrePARE is because that version of PrePARE lacks the "out_name" tests.

https://github.com/PCMDI/cmor/blame/master/LibCV/PrePARE/PrePARE.py#L299
https://github.com/PCMDI/cmor/blob/master/LibCV/PrePARE/out_names_tests.json

In CMOR 3.4.0, PrePARE looks up the variable and table name, extracted from the title of the NetCDF file, in out_names_tests.json. In this case it would find Omon_ficeberg and use the has_3_dimensions function to determine if the true name of the variable is ficeberg2d in CMIP6_Omon.json. The CMOR 3.3.2 version of PrePARE will match the variable name ficeberg with the ficeberg in CMIP6_Omon.json, causing the error with cell_measures.

@jonseddon
Copy link
Contributor Author

@mauzey1 brilliant, thanks for the clarification. Sorry, I hadn't spotted that there were separate ficeberg2d and ficeberg entries.

I'm happy to close this issue. @taylor13 that approach sounds good to me. Is there anything outstanding from your questions or are you happy for this to be closed?

@taylor13
Copy link
Collaborator

@jonseddon I agree your issue has been resolved.
@mauzey1 Do you think there is value checking the code for consistency with my "ideal behavior" outlined in #439 (comment) ? Would be great to hear what you think before closing.

@mauzey1
Copy link
Collaborator

mauzey1 commented Feb 13, 2019

@taylor13

  1. I understand omitting cell_measure if it's set to "-OPT", "-UGRID", or "-MODEL", or "". I don't get the part about throwing an error if "cell_measures" is found in the file. Is that a case where cell_measures is set to "cell_measures"?
  2. The current CMIP6 tables to don't have values like "area: areacello OR areacella" for cell_measures so the "if values" statement at https://github.com/PCMDI/cmor/blame/master/LibCV/PrePARE/PrePARE.py#L590 is ignored. Should we repurpose that statement to omit cell_measures when the value is "area: areacello OR areacella" or similar?

@taylor13
Copy link
Collaborator

taylor13 commented Feb 13, 2019

  1. sorry, I didn't say that clearly. I was trying to say the same thing in different words. Ignore the second sentence.
  2. No, since the latest CMOR version might be used with earlier tables, let's have it skip the check on cell_measures if it finds " OR " in the table's cell_measures string.

@taylor13
Copy link
Collaborator

@mauzey1 Can this be closed yet? What issues remain?

@mauzey1
Copy link
Collaborator

mauzey1 commented May 15, 2019

@taylor13

  1. It looks like PrePARE will currently ignore cell_measures if it finds "OPT" and "MODEL" but we could include "UGRID" and "" too.
  2. I couldn't find any table value where cell_measures has " NO " in its value. Should that still be include in the cell_measure values to ignore?

@mauzey1 mauzey1 added this to the 3.5.0 milestone May 15, 2019
@taylor13
Copy link
Collaborator

taylor13 commented May 15, 2019

Besides "OPT" and "MODEL",
PrePARE should ignore "cell_measures" if the table value is "UGRID".
PrePARE should ignore "cell_measures" if the table value is " OR " (for compatibility with earlier versions of the CMOR table).
PrePARE should raise an error if table value is "" but a cell_measure is defined in the file.

@mauzey1
Copy link
Collaborator

mauzey1 commented May 15, 2019

@taylor13 and also ignore any table value with "OR" such as "area: areacello OR areacella"?

@taylor13
Copy link
Collaborator

oops. In some of my previous messages, I meant to write "OR", not "NO". I've now corrected them. Yes, ignore any table with " OR ". Forget about anything to do with "NO", which as you noted isn't in any tables (and never was).
thanks.

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

Successfully merging a pull request may close this issue.

3 participants