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

One-side-limited slopes bug with MEKE_GM_SRC_ALT=True. #247

Open
Hallberg-NOAA opened this issue May 17, 2023 · 2 comments
Open

One-side-limited slopes bug with MEKE_GM_SRC_ALT=True. #247

Hallberg-NOAA opened this issue May 17, 2023 · 2 comments

Comments

@Hallberg-NOAA
Copy link

There is a bug in the application of KHTH_SLOPE_MAX to the slopes used to calculate the Potential energy released by the GM mixing for use by MEKE when MEKE_GM_SRC_ALT is true. Specifically the lines Slope_x_PE(I,j,k) = MIN(Slope,CS%slope_max) and its counterpart for Slope_y_PE (MOM_thickness_diffuse.F90, lines 928 and 1210) do limit positive slopes, but they do not limit negative slopes. As a result, this breaks rotational consistency, among other problems.

Obviously the solution to this bug is to replace this with something like Slope_x_PE(I,j,k) = MAX(MIN(Slope,CS%slope_max), -1.*CS%slope_max), but it could change answers for anyone using this option. To the best of my knowledge, we at GFDL are not currently using MEKE_GM_SRC_ALT in any cases, but because this code came from NCAR originally, it seems like this issue is best handled via dev/NCAR in case someone at NCAR is using it.

@Hallberg-NOAA
Copy link
Author

This bug is being addressed via NOAA-GFDL#620, which retains adds the new runtime parameter MEKE_GM_SRC_ALT_SLOPE_BUG to correct it, but leaves the bug in place by default. Because we are not yet using MEKE_GM_SRC_ALT = True at GFDL, we would not mind if this bug were just fixed in the code, but adding this bug-fix flag does seem like the right approach in case someone is using this option. With this bug-fix enabled, we are able to get rotationally symmetric solutions when MEKE_GM_SRC_ALT = True.

@iangrooms
Copy link

I don't think NCAR is using MEKE_GM_SRC_ALT = True right now. I played with that option when I was exploring MEKE configurations with no equilibrium-restoring, but ended up not using it. This discussion summarizes the options we're currently exploring. It's of course great that this bug is being fixed; my comment is just to say that the fix is not urgent for us.

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