Skip to content

Conversation

@Hallberg-NOAA
Copy link
Member

Replaced the 8 Coriolis parameter arrays ([abcd]zon and [abcd]mer) with merged 3-dimensional arrays (f_4_u and f_4_v), where the extra dimension is of size 4 and varies fastest. This reduces the number of arguments being passed around and it could improve performance by increasing the likelihood of cache hits. Also made the dgeo_de argument to set_up_BT_OBC() non-optional, which reduces the number of calls to this routine and should increase the chances it might be inlined. A few more spelling errors were corrected, and some notes added in comments highlighting potentially suspect reassignments of effective total gravities at open boundary condition points. All answers are bitwise identical and no external interfaces are changed.

@Hallberg-NOAA Hallberg-NOAA added the refactor Code cleanup with no changes in functionality or results label Mar 25, 2025
@theresa-cordero
Copy link

I would like the to confirm that the dimensions 1 to 4 positions (southwest, southeast, etc.) are correctly labeled in the comments. It would be good to clarify in the comments that f_4_u is not located at a u, v, q, or tracer point but instead at a quarter grid point and the positions are relative to the u point.

Otherwise, I am happy with these changes. I think they add clarity to[abcd]mer and [abcd]zon while adding clarity to the code and simplifying the interfaces.

@Hallberg-NOAA
Copy link
Member Author

Yes, @theresa-cordero, your interpretation here is correct about the actual staggering of these 4 variables. The previous variable names (a, b, c, d) date back to some classic papers by Akio Arakawa and collaborators, but is otherwise not very informative. The biggest change here is that these new arrays are now using the indexing of the points where they are used, whereas before the [abcd]zon variables used the indexing of the points where they are used for accelerations but the [abcd]mer variables used the indexing of the velocities whose contributions to the Coriolis accelerations they scaled. The new f_4_u and f_4_v variables are now always using the indexing of the accelerations they contribute to, which should make the code clearer.

  Replaced the 8 Coriolis parameter arrays ([abcd]zon and [abcd]mer) with merged
3-dimensional arrays (f_4_u and f_4_v), where the extra dimension is of size 4
and varies fastest.  This reduces the number of arguments being passed around
and it could improve performance by increasing the likelihood of cache hits.
Also made the dgeo_de argument to set_up_BT_OBC non-optional, which reduces the
number of calls to this routine and should increase the chances it might be
inlined.  A few more spelling errors were corrected, and some notes added in
comments highlighting potentially suspect reassignments of effective total
gravities at open boundary condition points.  All answers are bitwise identical
and no external interfaces are changed.
@Hallberg-NOAA Hallberg-NOAA force-pushed the barotropic_Coriolis_arrays branch from c48f770 to b01f9d3 Compare April 7, 2025 20:53
@Hallberg-NOAA
Copy link
Member Author

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/27053.

@Hallberg-NOAA Hallberg-NOAA merged commit 8be87a2 into NOAA-GFDL:dev/gfdl Apr 7, 2025
10 checks passed
@Hallberg-NOAA Hallberg-NOAA deleted the barotropic_Coriolis_arrays branch April 22, 2025 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code cleanup with no changes in functionality or results

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants