part of cam6_4_033: Wrap MPAS longitude values to [0,2pi) range #1095
Conversation
This commit addresses issues that were seen when using ncdata with longitudes in the [-pi, pi] range in cases with full physics. This would cause the atmosphere to become unphysical and runs to fail. This change is made just after the longitude values are read to ensure they are correct for the remainder of execution.
|
NOTE: This PR branch is based on cam6_3_148 for compatibility with EarthWorks. This can be updated at any time, in whatever way reviewers would prefer. Changes in this PR should also allow #1029 to proceed. |
NOTE: this is before the 'aux_cam' tests have been run
gdicker1
added
enhancement
Previous versions of the lonCell_arr adjustment used an explicit loop. Without this loop, the nCells variable and the routine to fetch it are no longer needed.
| call mpas_pool_get_array(meshPool, 'lonCell', lonCell_arr) | ||
|
|
||
| ! Ensure longitudes w/i [0, 2*pi) range | ||
| lonCell_arr(:) = lonCell_arr(:) - (2.*pii) * floor(lonCell_arr(:) / (2.*pii)) |
There was a problem hiding this comment.
| lonCell_arr(:) = lonCell_arr(:) - (2.*pii) * floor(lonCell_arr(:) / (2.*pii)) | |
| lonCell_arr(:) = lonCell_arr(:) - (2._RKIND*pii) * floor(lonCell_arr(:) / (2._RKIND*pii)) |
There was a problem hiding this comment.
Thanks for the reminder! Addressed by 288f3ed
|
@jtruesdal before I push to address your comment, I just noticed that I didn't wrap the commit message for f3dfa51 appropriately. In other repos, I would fix the message and force-push the branch. How should I proceed here? Just ignore it? |
|
I'm not sure what you mean by "wrap the commit message". Everything looks Ok to me when I look at the 3 commits and their associated messages. We mostly go by the Changelog and the commit messages are useful but not critical. |
Makes code consistent with CAM coding standards: 'All variables of type real must have a specified kind, including literals.'
|
Fair enough, I won't worry about it.
Most resources on git commit messages suggest to break body text at 72 characters and continue on the next line. |
There was a problem hiding this comment.
Besides giving some consideration to the two comments I've left, I think it would be worth using a loop with a conditional to only modify longitudes that aren't already in the interval [0, 2*Pi). It's ultimately likely to be slower, but this only happens once when the model starts up, and the cost is almost certainly negligible when compared with the cost of filesystem accesses. The benefit would be that, for meshes that provide longitudes in CAM's required range, those values would remain bitwise identical to their values in the input file.
This commit also extends the comment that was with this code block, includes an if condition so valid values aren't modified, uses kinds and constants (pi) that are already used in the new file, and updates the ChangeLog so the correct file is referenced.
Conform to CAM Coding Standards: 'Use symbolic numerical comparison operators (e.g., ==, /=, <, >=) not old character versions (e.g., .eq.).'
cacraigucar
changed the title
cacraigucar
changed the title
gdicker1
changed the title
This PR addresses issues that were seen when using ncdata with longitudes in the$[-\pi, \pi]$ range in cases with full physics. This would cause the atmosphere to become unphysical and runs to fail. This change is made just after the longitude values are read to ensure they are correct for the remainder of execution.
Fixes #1094