Remove unused vertical mixing schemes

[xylar]

Since we now only use CVMix, the rich vertical mixing scheme and perhaps others should be removed so we don't have to maintain code we don't use.

[vanroekel]

I agree with @xylar, we should remove the extra schemes. The functionality of all other schemes exists in cvmix. @mark-petersen if you agree, I could put this on my todo list to clean up.

[pwolfram]

@xylar, I'm very much against this because @mark-petersen, @vanroekel, and I used this to do debugging of the noise issue when ENSO was broken. I recognize it is painful to have this type of code in the repo, but not having the option to use does cause a potential problem.

This being said, if we were able to "swap" this out by replacing the functionality with a proper GOTM implementation, which is in ICoM and @qingli411 is starting to work with @vanroekel and myself, then this seems like the best approach forward because then we would just need to support the single GOTM vertical mixing API, which will give us CVMIX, @vanroekel 's new scheme, etc.

@vanroekel and @mark-petersen, please speak up if I've mangled something here. There may be some details in the GOTM/CVMIX implementation that may need sorted out.

[pwolfram]

@vanroekel, looks like we were writing at the same time-- we have P&P and Richardson mixing in CVMIX now? If so, I agree we can remove.

To clarify, the shift to GOTM is expected to include all the current functionality in CVMIX, right?

[pwolfram]

The bottom line is we need to maintain different physical vertical mixing schemes, minimally for debugging but also to have the science capability available for different cases. For instance, SOMA does not and should not use KPP.

[xylar]

@pwolfram, my understanding is that Richardson is fully supported in CVMix and there's no reason to use the MPAS implementation. If we still are, we shouldn't be because it hasn't been getting proper maintenance for a long time already.

[vanroekel]

@pwolfram, yes to both questions. Currently CVMix includes PP81 and LMD94 Ri mixing, it also has the Bryan and lewis constant mixing which can replace the tanh mixing, and certainly uniformly constant.

and yes once @qingli411 finishes incorporation of the new GOTM we will have all of CVMix plus the GOTM family of 2-equation models.

[vanroekel]

I'd also add that I'm not terribly convinced the testing we did with other vertical mixing schemes was helpful for the ENSO issue. However, the move toward all of this living in GOTM is a good one in my view.

[pwolfram]

The last question I have is how much will we trust / be sure we don't have latent bugs in the CVMIX / GOTM implementations? Ideally no, practically probably. If so, assuming Richardson still works, we may want to keep it around to make sure we are at least qualitatively consistent with CVMIX.

For example, if we only have one mixing scheme via GOTM/CVMIX, what happens if there is a hidden bug in this implementation? We won't have an easy way to cross-check it against an existing set of tested code.

@vanroekel, I agree, the vertical mixing testing wasn't too helpful-- but, that was because it wasn't the issue. One can imagine multiple scenarios where this could be a real problem that produces a pernicious bug in at least the ocean or likely the coupled system.

We can alleviate these concerns via some type of unit testing for CVMIX/GOTM, but to the best of my knowledge we really don't have this type of testing infrastructure or approach and bugs in CVMIX that have come up have appeared to be solved more on an ad hoc basis, meaning we don't have solid coverage for certainty that the interface is completely working. If this is the case, playing devil's advocate, then we shouldn't remove Richardson.

This is a question about the trade off between maintenance, which could be highly onerous as more code changes are made (especially for performance), and testing of existing vertical mixing schemes, of which there are probably better approaches that are already in place or could be put in place.

[pwolfram]

And my personal take on this is I would like to see all our mixing be in GOTM with the CVMIX hook under a testing infrastructure that makes us fully comfortable under the scope of science we support and will do, recognizing no testing suite is ever 100% and we will have to hold some degree of risk to be successful here. In this "world", we don't need ad-hoc mixing scheme implementations because these make our code look "hacky" and make it harder to collaborate with others.

[vanroekel]

The current RICH / TANH / CONST mixing code does not give me any comfort or a decent test case for the CVMix implementations. All three of those are trivial mixing schemes to implement. The most complex code in CVMix by far is KPP. The others are simple polynomial formations fixed in time and I don't see how maintaining them helps debug cvmix. I would also strongly disagree that the debugging of the CVMix interface has been "ad hoc". It was a detailed comparison vs. LES and against the LMD94 "theory". I am highly dubious that a unit testing framework would have been better/more efficient. So, In my view continuing to maintain the current non CVMix code is much more onerous than any benefits it may have.

I'm not sure how I feel about all of our code being in GOTM for mixing eventually. There are other issues to consider, e.g. performance. But where mixing code lives ultimately is far afield from this issue.

[xylar]

@pwolfram, I'd like to chat a bit offline about this before continuing the conversion. Please respond on Slack if you get a chance.

[pwolfram]

Thanks @xylar and @vanroekel my concern was related to various bugs you've found and described to me in CVMIX over the years, especially related to the vertical indices, empty or incorrect values, etc. I wasn't suggesting the process to debug it was done incorrectly and to be fair have said and continue to think your analysis is perhaps the most comprehensive one to date. I am not concerned with the science or its embodiment in code here. I'm concerned about unknown unknowns and having tools to protect us against this risk.

The concern, if any, is whether our existing knowledge is formalized in a regression-suite style, full-coverage (if such a thing is possible) test suite that someone without detailed knowledge can run and use to protect the good work that has already been done and as a debugging tool when more complex simulations we run have issues (e.g, to make sure changes don't break this core functionality).

Given we don't have robust vertical mixing testing outside CVMIX, we can remove that code and move onward but this doesn't alleviate the broader concern of more comprehensive testing (which applies to other parts of the code not discussed here too).

For instance, if I change something in MPAS-O and the existing CVMIX test case is no longer BFB, do I have a clear idea of what went wrong just from the test case failing? I recognize this is a high bar but this is my point-- for a critical piece of the code like this it would be good to have this certainty in terms of testing if possible, especially as we consider doing more complex things like going from deep to shallow water and vice versa. This will be especially needed to help facilitate performance work but perhaps the BFB approach we have now is enough. Perhaps a more complete testing suite is too impractical given other constraints. Likely there is a middle ground to find here that makes sense unless the answer is we are already confident enough in the automated testing. Does this make more sense?

My fundamental concern is how do we know the code we have works and is robust to other changes in the code in terms of some type of automated confirmation? For instance, if we change the vertical mesh, could we qualitatively make sure we don't somehow break the vertical mixing? A follow-on question depending upon the answer would be, "Does this code need generalized and put on the development timeline as fundamental to other science questions?" These are the types of questions I'd like to see readily answerable or at least informed in terms of a clearer "I ran a script and it passed/failed" type result. The concern is obviously much more general than vertical mixing and readily extends to other code like wetting / drying, online diagnostics, etc.