Use safer default min_thickness = 1.0e-3

[aekiss]

see COSIMA/access-om2#161

[aidanheerdegen]

@russfiedler I've pinged you on this just so you can confirm this will have no unintended consequences. Seems harmless enough, but thought it good to check.

[russfiedler]

It may change users' high vertical resolution grids if they are poorly specified and rely on the old defaults. New experiments will be fine but starting from existing restarts may have problems since the code as it stands does not do what is intended and may have produced a corrupted kmt mask.

[aidanheerdegen]

So arguably it is a big fix that may break some existing restarts? Is there a work-around in those cases?

[russfiedler]

The user can set the value in the namelist.

[aidanheerdegen]

In which case, I reckon we should just make this change.

[aekiss]

If this change breaks runs from old restarts, will it appear to run OK or will runs fail so users know there is a problem?

[russfiedler]

The kmt mask will have extra cells which were formerly rock(but the user probably thought were ocean). It should crash the new runs. The only way that I can think of protecting the user is to demand that they supply a valid min_thickness. Have a negative default and abort if it is negative with messages explaining how to safely choose a sensible value and why. Maybe that's a better option.

[StephenGriffies]

If there is a crash for new runs due to changes in default, then we need to tell users (via error message) what they should set to accord with the prior defaults. Setting a "valid min_thickness" begs the question what is valid and what was the setting prior to the hard error.

[aidanheerdegen]

Sorry, I merged this PR before @StephenGriffies had a chance to comment. I'm happy to submit another PR with Steve's suggestions, or @russfiedler can knock something up.

If I'm to do it, I'd appreciate some help in terms of where this error message should go in the code and what criteria to use to diagnose this issue.

[aekiss]

how does this look? #309

[russfiedler]

I'd add that for floating point representation reasons min_thickness should be no less than spacing(max_depth,kind=real32) where max_depth is the maximum depth of the model and the real32 parameter can be obtained from iso_fortran_env.

[aekiss]

ok thanks @russfiedler, I'll add that.
Also the documentation states that min_thickness is only used for a mosaic grid but we terminate without checking for that, ie we demand that min_thickness is specified even if unused. I guess that's ok? Or should we check we are on a mosaic grid before deciding to terminate?

[russfiedler]

min_thickness is also used if kmt_recompute=.true. (not recommended). So I'd only check it in the file_exist(ocean_topog) and kmt_recompute if blocks.

[aekiss]

did you mean something like this? https://github.com/mom-ocean/MOM5/pull/309/files

[russfiedler]

Be careful with the length of the error message. I think it's limited to 512 characters.

[aekiss]

ok I've now reduced the messages to under 512 chars

[aekiss]

Any objections to me merging the updated PR #309 into master?

Note that this will break old configs (by design), as it now requires min_thickness to be explicitly specified in ocean_topog_nml.

As explained in the error message, old configs should set min_thickness to 1.0 to restore their previous behaviour, whereas for new runs min_thickness=1.0e-3 is a better choice.