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

Refactor PressureForce_FV before ice shelf fixes #629

Merged

Conversation

Hallberg-NOAA
Copy link
Member

Refactored PressureForce_FV_nonBouss() and PressureForce_FV_Bouss() to use more 3-dimensional arrays in preparation for the changes that we are expecting from Claire Yung to reduce the pressure gradient errors in ice shelf cavities. These anticipated changes will involve selecting an internal interface at which to define the shape of the interfaces with depth when there is an ice shelf, rather than assuming that the top surface pressure varies linearly with depth between the two top corners. In PressureForce_FV_nonBouss(), this refactoring includes turning za, intx_za and inty_za into 3-d arrays and moving the code setting these arrays outside of the k-loop setting the pressure gradient forces. Changes in PressureForce_FV_Bouss() are analogous but involve turning 7 arrays (pa, dpa, intz_dpa, intx_pa, intx_dpa, inty_pa and inty_dpa) into 3-d arrays. In both cases, the code for a reduced gravity model is consolidated into a single block toward the end of the routines. These changes to use more 3-d arrays could increase the computational time for the pressure gradient calculations, but in short regression tests any such increase in computational time appears to be small. No external interfaces are changed, and all answers are bitwise identical.

  Refactored PressureForce_FV_nonBouss and PressureForce_FV_Bouss to use more
3-dimensional arrays in preparation for the changes that we are expecting from
Claire Yung to reduce the pressure gradient errors in ice shelf cavities.  These
anticipated changes will involve selecting an internal interface at which to
define the shape of the interfaces with depth when there is an ice shelf, rather
than assuming that the top surface pressure varies linearly with depth between
the two top corners.  In PressureForce_FV_nonBouss, this refactoring includes
turning za, intx_za and inty_za into 3-d arrays and moving the code setting
these arrays outside of the k-loop setting the pressure gradient forces.
Changes in PressureForce_FV_Bouss are analogous but involve turning 7 arrays
(pa, dpa, intz_dpa, intx_pa, intx_dpa, inty_pa and inty_dpa) into 3-d arrays.
In both cases, the code for a reduced gravity model is consolidated into a
single block toward the end of the routines.  These changes to use more 3-d
arrays could increase the computational time for the pressure gradient
calculations, but in short regression tests any such increase in computational
time appears to be small.  No external interfaces are changed, and all answers
are bitwise identical.
@Hallberg-NOAA Hallberg-NOAA added the refactor Code cleanup with no changes in functionality or results label May 12, 2024
Copy link
Member

@marshallward marshallward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Gaea, I saw a 5% slowdown with Intel and 6% slowdown with GNU.

On my laptop - which has very low specs - the GCC slowdown was much higher at 20%. But I would not say this is the sort of machine that we are targeting.

As long as we're comfortable with those numbers, then I can merge this one.

@marshallward
Copy link
Member

Double checked these numbers on my RAM-heavy home desktop. Again observed 6% slowdown on GCC.

I think the 20% is likely just cache swapping, which we can put under "Minimum Hardware Requirements".

I'm not so sure about the 5% slowdown. I could guess, but I'll resist for now. We've agreed to merge this in and open an issue to investigate when it makes more sense.

@marshallward
Copy link
Member

@marshallward marshallward merged commit 215b960 into NOAA-GFDL:dev/gfdl Jun 7, 2024
10 checks passed
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