Fixing issue #539

[kshedstrom]

There are two places in the code where USE_BODNER23 is checked. They have to be consistent. This fixes the inconsistency I had mentioned in #539 , but does introduce an extra

MLE%
%MLE

into the output doc files.

[marshallward]

I don't think this patch is quite doing what we want. From my reading, it's implicitly creating MLE%MLE%USE_BODNER23, which is equivalent to

MLE%
MLE%USE_BODNER23
%MLE

I think the only real fix here is to get the parser to properly read the contents of MLE% ... %MLE.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a03bb95) 37.20% compared to head (b272fc1) 37.20%.

Additional details and impacted files

@@            Coverage Diff            @@
##           dev/gfdl     #540   +/-   ##
=========================================
  Coverage     37.20%   37.20%           
=========================================
  Files           271      271           
  Lines         80355    80357    +2     
  Branches      14985    14985           
=========================================
+ Hits          29894    29897    +3     
  Misses        44903    44903           
+ Partials       5558     5557    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kshedstrom]

No, this properly parses

MLE%
USE_BODNER23
%MLE

[marshallward]

Ah sorry.... I think I confused myself. Maybe that's why cs%use_bodner was never set.

@adcroft Does this look ok?

(Ed: I see why I was confused now: I thought you had wrapped MLE%USE_BODNER23 in the MLE block; missed that you had removed MLE% from the parameter name.)

[marshallward]

I've checked this in tc2.a and both forms work and give the same answer with this patch.

[marshallward]

I spoke with @adcroft and he agreed that this is the preferred method of accessing parameter block contents.

There was a thought that get_param(..., "MLE%USE_BODNER23", ...) still ought to work, despite some potential ambiguity of the "top" parameter block (e.g. what if we were inside KPP%?). But it's clearly broken as-is and also not the preferred method, so we will just leave it for another time.

Our test machine Gaea is being turned upside down today, but I'll try to get this tested and merged ASAP.

[marshallward]

We think there still may be a minor issue with the new open/closeParameterBlock function calls. This new solution appears to be printing an empty MLE%/%MLE block in the MOM_parameter_doc.all files.

At the least, we might need to add support for do_not_log= to openParameterBlock. We might also want to consider further restructuring of the parameter blocks.

[marshallward]

#558 should eliminate the redundant MLE% %MLE block records.

[marshallward]

I've run a modified version of this PR which appears to fix the problem and does not produce a redundant MLE%/ %MLE. It is now passing our tests with no parameter regression.

I will merge this directly to MOM6 and close this issue once it's been finalized.

---

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/22146 ✔️

[marshallward]

Matching commit: 5ca70ba