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

CIME calls cprnc in a non-symmetric way #4303

Closed
bartgol opened this issue Sep 12, 2022 · 4 comments · Fixed by #4304
Closed

CIME calls cprnc in a non-symmetric way #4303

bartgol opened this issue Sep 12, 2022 · 4 comments · Fixed by #4304
Assignees

Comments

@bartgol
Copy link
Contributor

bartgol commented Sep 12, 2022

I changed the output precision of a test from double to float. I was expecting baseline tests to fail, but our tests still passed.

Intrigued, I manually ran cprnc and indeed it reported 0 diffs. However, flipping the files order did show the difference. I'm guessing cprnc chooses the precision from the 1st file (which, for me, was float), so even if the second file has higher precision, it does not see that.

Is this desired? Should CIME do a symmetric test instead? Or is this too much of a corner case to be worth cime mods?

@jedwards4b
Copy link
Contributor

I can reproduce this issue. I think that it should do a symmetric test.

@jedwards4b jedwards4b self-assigned this Sep 13, 2022
@bartgol
Copy link
Contributor Author

bartgol commented Sep 13, 2022

It would be enough to use the highest precision between the two fields. Or maybe check that the two have the same precision, and return a diff if they don't (the likelihood of a double field to store the same values as a float field is probably very tiny anyways).

@jedwards4b
Copy link
Contributor

Here is my proposed solution
If the lower resolution data type is in the first file:

SUMMARY of cprnc:
 A total number of    427 fields were compared
          of which      0 had non-zero differences
               and      0 had differences in fill patterns
               and      0 had different dimension sizes
               and      1 had different data types
 A total number of      0 fields could not be analyzed
 A total number of      0 time-varying fields on file 1 were not found on file 2.
 A total number of      0 time-constant fields on file 1 were not found on file 2.
 A total number of      0 time-varying fields on file 2 were not found on file 1.
 A total number of      0 time-constant fields on file 2 were not found on file 1.
  diff_test: the two files seem to be DIFFERENT 

If it is in the second file:

SUMMARY of cprnc:
 A total number of    427 fields were compared
          of which      1 had non-zero differences
               and      0 had differences in fill patterns
               and      0 had different dimension sizes
               and      1 had different data types
 A total number of      0 fields could not be analyzed
 A total number of      0 time-varying fields on file 1 were not found on file 2.
 A total number of      0 time-constant fields on file 1 were not found on file 2.
 A total number of      0 time-varying fields on file 2 were not found on file 1.
 A total number of      0 time-constant fields on file 2 were not found on file 1.
  diff_test: the two files seem to be DIFFERENT 

In either case additional information is available in cprnc output:

 WARNING: Variable Med_alb_ocn_So_avsdf types differ
 TYPEDIFF Med_alb_ocn_So_avsdf                       6           5

@bartgol will this meet your needs?

@bartgol
Copy link
Contributor Author

bartgol commented Sep 13, 2022

Yes, I think it's great. Thanks!

jedwards4b added a commit that referenced this issue Sep 13, 2022
add a data type check to cprnc

Add a check to assure that data types are the same in compared files.

Test suite: by hand, ./scripts_regression_tests.py test_sys_cime_case.test_self_build_cprnc --compiler gnu
Test baseline:
Test namelist changes:
Test status: bit for bit

Fixes #4303

User interface changes?:

Update gh-pages html (Y/N)?:
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.

2 participants