-
Notifications
You must be signed in to change notification settings - Fork 132
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
Precision #446
Precision #446
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How were the pio changes tested in pio? Was binary mode tested?
Could we change set_nml.precision to set_nml.precision8 and then update the base_suite.ts testlist
Could the changes in cice.build, Macros.cheyenne_intel, and env.cheyenne_intel be reverted. I don't think they are needed for this PR and they will conflict with changes being made to implement standalone pio testing in a separate PR.
I was not able to get the pio interfaces working. I am just putting in the precision changes so we have them. It works with binary and netcdf. I can back out the Macros and env changes, but this is what I did to try to get everything to compile. Is this not the way we want to go? |
The Macros and env changes I've made are similar, but not quite the same. Nothing wrong with what you have, but my mods are slightly different. It would be easier for me if you removed your changes. |
Ok. Changes made. |
There is also a change in cice.build that could be reverted if you have a chance. thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One other thing, the error check at line 516 in ice_init should be moved down in the subroutine and then it should set an abort flag. The idea is that we want all the namelist errors to be written out after parsing then for the model to abort. If it aborts one error at a time, it can take several runs to work thru the errors.
Is there any systematic way that the abort_flag number is set? Why does it jump from 21 to 30? |
The abort_flag number is not important as long as it's not zero. It's better if it's unique. In fact, as presently implemented it will be the last error encountered if you have multiple. It would be better for this to be a character string that somehow accumulates the error list, but I don't think that's important to do at this point. The main point is that the errors are all written then later the code aborts if 1 or more is found. The abort_flag could be a logical too. My making it a number, I thought it would provide more info, and it does, but not as much as it might. Thanks! |
One more dumb question. Why are there two abort_flag settings done after the print statements? |
Not sure, I suspect it's historical. Some checks may have been originally done before the namelist is written to the log files and others after. That order probably was not changed when the abort_flag was implemented. We could probably clean that up a bit, but I'm not sure it's important. Actually, the kevp_kernel flag (21) probably should remain after the output because we want to see the original setting in the output before it's potentially changed. abort_flag=20 could be moved up or they all could be moved down. |
I think this is ready to merge. @dabail10 any outstanding issues? @eclare108213 do you want to have a review? I am a little eager to merge this so I can pull it into my pio standalone testing and address any conflicts or other issues. If we want to wait a bit to merge this, I can wait (or I could merge from this dev branch to do some initial testing). |
For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers
PR checklist
Add history_precision namelist option.
dabail10 (D. Bailey)
Test-Results.wiki/cice_dev/c38a514361.cheyenne.intel.20-05-14.154337.0.md
The history file answers will only change when turning on double-precision history output, that is history_precision = 8.