search

mom-ocean / MOM6

Modular Ocean Model
Other
185 stars 228 forks source link

USE_QG_LEITH_VISC=True uses code with numerous bugs #1590

Open Hallberg-NOAA opened 1 year ago

Hallberg-NOAA commented 1 year ago

The QG Leith viscosity option in MOM6 has serious problems that need to be corrected before it is usable.

Specifically, when USE_QG_LEITH_VISC=True, MOM6 does not reproduce across PE count or layout, and fails the dimensional consistency testing. Moreover, this option is using arrays with stored slopes that are only calculated if THICKNESS_DIFFUSE=True and USE_STORED_SLOPES=True, and one of KHTR_SLOPE_CFF>0 or KHTR_SLOPE_CFF>0 or MEKE=True. Moreover, these slopes will only be updated after they used for the horizontal viscosity if THICKNESSDIFFUSE_FIRST=False, which will break reproducability across restarts and may be completely inconsistent after an ALE vertical remapping step. In other words, this option fails to reproduce across PE count, restarts, or dimensional rescaling, and it may give random numbers during the first time step such that repeated runs of the code do not give the same answers.

I have a code fix in hand that corrects the dimensional rescaling issues (without changing answers much), but addressing the other issues may be more of a challenge.

How we deal with these problems may depend on whether anyone is actually using this code. Given all of these problems, I can not imagine that USE_QG_LEITH_VISC=True is not being used in any runs that anyone cares about, and that it was just added as the rough-draft of an idea. If (as I sincerely hope) no one is using this code option, we will have leeway to make the necessary corrections to address all of the issues described above. On the other hand, if someone is using this option, we have a serious problem with wide-ranging consequences.

In the mean-time, I am inclined to temporarily add a fatal error if anyone tries to use this code to avoid creating any serious issues by anyone who unwittingly uses this code.

I would appreciate a response from anyone who is using this code so that we can figure out what to do about these problems. In particular, @sdbachman and @gustavo-marques, you contributed or worked on this code (about 4 years ago) and probably are in the best position to know whether this option is actively being used and by whom.

sdbachman commented 1 year ago

We aren't using this option to my best knowledge, and I don't think anyone else is either. I would not be bothered if police tape was put up around this code for now.

Hallberg-NOAA commented 1 year ago

Thanks for the quick response, @sdbachman. I have a PR pending via dev/gfdl at https://github.com/NOAA-GFDL/MOM6/pull/314 that includes a fatal error message if anyone tries to use the problematic option.

Hallberg-NOAA commented 1 year ago

I have a draft commit that corrects the reproducibility issues when USE_QG_LEITH_VISC=True at https://github.com/Hallberg-NOAA/MOM6/commit/567f7bd808e595eebd92b8d5ddfe2cab7562a1ce . However, this commit involves sufficiently widespread (albeit minor) changes to public interfaces and the potential to slightly degrade the computational performance of cases that are not using USE_QG_LEITH_VISC=True (because of its expanded halo updates), that I think that further discussion is warranted about how to handle it.