-
Notifications
You must be signed in to change notification settings - Fork 2
Double precision option for regridded SMC output in ounf #52
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
Conversation
Please note this is currently only implemented for SMC grids types 2, 3 and 4.
d34474f to
6a84480
Compare
ukmo-kitstokes
left a comment
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.
Hi Daisy, great work on getting this PR submitted :). I've reviewed most of the code and am happy, with some minor comments provided. There are a number of files with too many lines to display on Github web browser. I have a feeling these are possibly just empty lines as there are some other files with empty lines in. These may all just need tidying up to remove unnecessary diffs occurring.
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.
lots of empty lines added, probably doesn't make any difference but maybe worth deleting so there's no diff showing.
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.
There it is!
|
Hi Daisy, can you attach examples of level 1 re-gridded output from our global model run and from our NWS (rotated) domains please? That will help confirm that the changes have had the desired effect on our wave output coordinates. |
|
Hi @ukmo-kitstokes please see the attached. Note there appears to be a very slight deviation in the base ten string printouts around 0 on UK domain's longitude; this is to be expected and should not be a concern as we discussed at the time; it does not represent any true loss of precision. |
|
Hi daisy, I'm happy with the evidence that the coordinates have been printed with the correct grid step and precision seems to be maintained throughout. Nice work getting working and tested. We should document somewhere the fact that the coordinate precisions are quite sensitive to compiler optimisation settings, and which settings are recommended to get coordinate precisions as expected. This could be worth adding to the ww3 manual (if and when we decide to put something in there about the DOUBLE_COORD switch) but should definitely be made explicit in your comments in our build scripts so that it's not removed down the line. I'm now happy for this PR to be merged into our fork. |
|
Thanks Kit, I'll get a change to the build scripts pushed up to our WW3BUILD git repository along with a comment about this. I'll have to look into where/how we make changes to the WW3 manual. I'd be inclined to do slightly more testing with tweaks to the grid and turning double precision on/off just to properly understand when having double precision on type 2 is useful, because the 'base' step size and starting positions defined by the grid should still be being defined as singles if my current understanding is correct. So it's still somewhat of a mystery to me why this whole thing works for us - and which situations for others it would work in, which we would have to specify in manual. In terms of implementation though it's about as much as we can do I think so I'm happy with it being merged for us. I'll need an approving review to mark this pull request as merged and put it straight into develop. |
Pull Request Summary
A namelist option to provide double precision output for type 2, 3 and 4 SMC grids.
Description
This pull request adds a .nml option SMC%DOUBLE_COORD to the ounf namelist files which will produce a regridded output as a 64 bit double rather than a 32 bit single floating point number. This is likely to be useful where a step size cannot be stored exactly in floating point but double precision is necessary for other components of a workflow.
Please note that type 3 and 4 grids are only supported by our internal operutils; see 737e352.
Currently only .nml is supported; .inp files will default this option to false. This decision has been made because supporting .inp without any tricky pattern matching which may cause more issues in future would mean that all systems using SMC grids would have to update any .inp files being used regardless of benefit to their use case. It would also require changing the relevant regression tests.
Issue(s) addressed
Commit Message
Add double precision option to ounf namelist
(co-authors: @ukmo-kitstokes)
Check list
Testing
The new version has been tested with regression tests on cray_gnu on EXCD.
matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt
An unexpected change appears within the binary model definition files, of 1 byte between each version, for some tests. It appears that this is caused by the variable XFT which is written to the file and defined in w3gridmd; however is only initiialised when the ST2 switch is set. In tests where ST4 switch is set the variable is thus uninitialised and gives unexpected changes; this does not indicate any issue with these commits or the feature, and it has not caused any bit differences in later files. Thus I consider these tests to be passed based on the version tested in this PR.
2 new directories were added to the Great Lakes regression test (mww3_test_09) which run with this option activated, and run as expected.