Plume

[bolding]

We should get this merged in before we make a new release.

Comments, ideas or suggestions?

What do these do? logical, parameter :: plume_surface=.false. !NEW_HB logical, parameter :: plume_bottom =.false. !NEW_HB

[hburchard]

logical, parameter :: plume_surface=.false. !NEW_HB logical, parameter :: plume_bottom =.false. !NEW_HB

I had put in those into the code as a hard switch.

Those have to be read in from gotm.yaml an should both be set to zero on default. For a surface attached plume (e.g. under glacier ice) it should be plume_surface=.false. and for a bottom attached plume it should be plume_bottom =.false. Only one of them should be set to true.

Ciao, Hans.

[bolding]

but they are logical and parameters - so 1) they can't be set to 0 and 2) they are parameters so they can't be read from a file

[jornbr]

And if they are mutually exclusive, could we not have just a "plume" settings that takes "off", "surface", "bottom" in gotm.yaml?

[hburchard]

I think we can do so.

[jornbr]

The changes are currently breaking backward compatibility - see the failed tests. That seems to be because /mimic_3d/int_press/dtdx and the like are no longer recognized. Can this be fixed? Otherwise the merging in of plume would require a major version boost, to GOTM v7.x

[bolding]

but is that not what we do when we go into a new devel branch. The internal pressure now have two different options - gradients and plumes - each of those have additional settings. 

the gradient specifications are one level deeper than 'gradients' it self - and similar with 'plume' and I think that shall be reflected in the layout of the configuration file

and if we update to 7 - we can use 7 until we make a release again in the distant future

[jornbr]

The alternative would be to, for the sake of backward compatibility, not introduce the additional "gradients" nesting ("plume" could stay). That would preserve backward compatibility, meaning the next version with plume support could be v6.x, and the new binary would still run all v6.0 setups.. Otherwise every single GOTM user will have to rerun update.py for their setup as soon as they pull in the plume changes - not that friendly... Which reminds me - did you update update.py? :-)

[bolding]

I'm a purist and prefer the right indentation at the expense of backward compatibility :-)

And no - this is a code review and update.py will not be changed before we know the outcome of the pull request discussion ....

[jornbr]

Re updating update.py - later is fine, but I do think it should happen before the PR is merged in.

[jornbr]

Also, I think we should set up the testing such that we can require each PR to pass all tests before it is merged in. That would mean making gotm-cases a submodule of gotm (because "plume" would require updates of all cases due to the moving of gradient settings; those updates should likely go into a new gotm-cases branch "plume").

[bolding]

I agree with the update of update.py must be done before the merge

can the GitHub Actions clone of gotm-cases be setup to reflect the code branch being tested

[jornbr]

Yes, for the testing you could also change .github/workflows/gotm.yml: add ref: <BRANCHNAME> under the with section of the gotm-cases clone.

[bolding]

The plume branch has been merged in