Changes to richardson number calculation

[vanroekel]

This changes the calculation of the gradient and bulk richardson numbers to utilize the normal and tangential velocities to avoid the factor of two issue and also fixes a number of places where the loop was over maxLevelCell and should have been to maxLevelEdgeTop

[vanroekel]

@maltrud @golaz @jonbob @alicebarthel @mark-petersen this the second PR I mentioned. I have not yet tested the full build of this separated from the other PR -- #65 and will report back soon on that testing but wanted to post this for initial examination and review

Same note as in the other PR this testing has always included #53

[vanroekel]

Confirming this builds okay and is ready for anyone to test.

[mark-petersen]

Testing now. This successfully compiles standalone with OPENMP=true on chrysalis with gnu and intel.

[dengwirda]

The purpose of these changes is to investigate whether a grid-scale mode may be present in Ri --- currently vmix tendencies in MPAS-O (standalone) show horizontal checkerboarding, espec. wrt. the convective trigger which shows strong unexpected oscillations in the deep(er) ocean.

The changes here are:

• Evaluate N^2 on edges rather cells only, to introduce some horizontal coupling. ROMS includes a Laplacian smooth of Ri in both the horizontal + vertical directions to guard against grid-scale checkerboards, whereas MPAS-O currently includes a 1-2-1 filter in the vertical-only. The horizontal averaging here should introduce a similar horizontal smooth.
• A change in shear calc. at bathymetry, to ensure there's not du/dz computed into a masked edge --- this may have been computing spurious shears at the ocean bottom in the old formulation.
• Use u and u^perp rather the average-of-dot-products-with-factor-of-2 alternative, since the later appears to be 0th-order accurate on non-uniform meshes, similar to the kinetic energy treatment.

@vanroekel @jonbob @maltrud @alicebarthel @mark-petersen and all, I suggest intercomparing against POP as a way to get to the bottom of this

[mark-petersen]

With the current head of this PR I still get

ocean/global_ocean/QU240/WOA23/decomp_test
  * step: 4proc
  * step: 8proc
  test execution:      SUCCESS
  test validation:     FAIL

with gnu debug on chrysalis. There must be something needed for halos, in addition to the last commit.

[vanroekel]

Hmm, that’s unfortunate, but I guess I never did do an official decomposition test, just reduction of eyeball norm noise.

Looking through the code, it seems the possibilities are halos on

1. Normal velocity
2. Tangential velocity
3. Layer thickness
4. Brunt vaisala frequency
5. Zmid
   All of these seem to have all halos (computed to nedgesall and ncellsall) on first look so I’m stumped. I’ll keep looking

[vanroekel]

@mark-petersen an update. I've tried to run the decomp test a bunch of times and have turned off cvmix, GM, and redi. This PR only should affect RiTopOfCell and bulkRichardsonNumberShear, which if I turned these things off it should run like master. But the test still fails. Do you have any suggestions? I'm wondering if my build has inadvertently diverged with the PR, I'll check that first, but if you have other ideas, let me know.

[vanroekel]

@mark-petersen an update -- I got my decomp test to give some more helpful info. I've narrowed it down to the bulkRichardsonNumber shear portion of the code mods. The test passes if config_use_cvmix_kpp = .false.. It's not yet clear to me what is wrong with the code in the cvmix coeff_build, but getting closer

[vanroekel]

@mark-petersen I found a fix for this issue and fixed another bug. Can you test? If it works for you we need to add a halo exchange on tangential velocity in RK4 as well

[mark-petersen]

Great! ocean/global_ocean/QU240/WOA23/decomp_test now passes the internal comparison with gnu debug. I will continue my review.

[mark-petersen]

@vanroekel, ocean/global_ocean/QU240/WOA23/RK4/decomp_test fails the internal comparison. Is there a computation or halo exchange that is outside the tendency routine?

All other nightly suite tests pass gnu debug on chrysalis.

[vanroekel]

@mark-petersen I forgot to add a halo exchange in RK4 that I added to split explicit. I'll make sure all time steppers get it now.

[vanroekel]

okay I think it should pass that RK4 test now.

But a question, I notice that in split_explicit/implicit there is a surface friction velocity exchange right before what I added that doesn't appear in RK4, should it? or is that halo exchange not necessary?

[mark-petersen]

Now passes nightly suite with both gnu debug and intel debug.

[mark-petersen]

Based on the above discussion and testing, this is now ready to move to E3SM-Project/E3SM repo.

[mark-petersen]

Moved to E3SM-Project#5946

[xylar]

@mark-petersen, can we delete this branch on Ocean-Discussion?

[xylar]

Question should probably have been to @vanroekel. I'm going to delete these branches because I don't think they are needed on Ocean-Discussion. In general, we want to open PRs here form branches from people's forks, not from this fork.

[vanroekel]

Sorry for that mix up @xylar and thanks for deleting those branches