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

Dimensional inconsistencies in vertFPmix() #259

Open
Hallberg-NOAA opened this issue Oct 28, 2023 · 1 comment
Open

Dimensional inconsistencies in vertFPmix() #259

Hallberg-NOAA opened this issue Oct 28, 2023 · 1 comment

Comments

@Hallberg-NOAA
Copy link

There are at least 6 expressions in the recently added routine vertFPmix() that are dimensionally inconsistent or are inconsistently implemented between the u- and v- components. These include things like the use of GV%H_subroundoff (with units of [H ~> m or kg m-2]) to avoid division by zero by adding them into denominators with units of [L2 T-2 ~> m2 s-2] on line 440 of MOM_vert_friction.F90, but in other cases they arise from omitted parentheses (e.g., (dt/CS%h_u(I,j,k) + GV%H_subroundoff) instead of (dt/(CS%h_u(I,j,k) + GV%H_subroundoff)) on line 460 of MOM_vert_friction.F90).

The PR to dev/gfdl at github.com/NOAA-GFDL/pull/510 includes comments highlighting the dimensionally inconsistent expressions.

These inconsistencies were detected by visual inspection of the code when we were adding comments documenting the units of the newly added real variables. However, these dimensional inconsistencies should also be detectable in any code that is being exercised by checking whether answers are bitwise identical for any values of the 8 run-time dimensional rescaling factors given by [HLTRZCSQ]_RESCALE_POWER between about -140 and 140 (i.e., rescaling various basic units by factors between 2^-140 ~= 7.2e-43 and 2^140 ~= 1.4e42).

In addition to using these dimensional rescaling tests to detect dimensional inconsistencies in any new code contributions, it would also be very helpful if comments were added describing the intended units of any real variables. We are trying to enforce this for any real arguments, but it would also be helpful if the same applied to internal variables, noting that [various] or [arbitrary] units may make sense in some cases.

The right strategy for correcting these bugs may depend on whether the new routine vertFPmix() is being used yet in any published or otherwise important simulations that need to be preserved with a run-time bug-fix flag. Because this has only recently been introduced to the main branch of MOM6 from dev/NCAR, I think that it is up to the NCAR MOM6 development team to decide how to handle them.

@gustavo-marques
Copy link
Collaborator

Thanks for reporting this, @Hallberg-NOAA. We are still working on vertFPmix. We will fix the inconsistencies you identified as part of the ongoing development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants