SplitExplicitFreeSurface code improvements

[simone-silvestri]

I want to propose a couple of improvements to the SplitExplicitFreeSurface code that might be useful for later extension of the time-stepping scheme.

(1). The code for the SplitExplicitFreeSurface is very long and crammed into only two files, making it very difficult to navigate. I want to split it into more files to make it easier to target modifications, possibly giving it its own module that lives in the HydrostaticFreeSurfaceModels module.

(2). At the moment the tendencies of the barotropic velocities are stored in a type called SplitExplicitAuxiliaryFields that is a field of the SplitExplicitFreeSurface type.

I want to remove this type because, after #3841 that removes the column depths, this type only stores the slow barotropic tendencies and the kernel parameters.

• The kernel parameters are not SplitExplicitAuxiliaryFields
• The barotropic velocities are actually prognostic fields of the model, so the slow tendencies should be placed where all other tendencies are (i.e. in the main timestepper of the HydrostaticFreeSurfaceModel). As a consequence of this, hydrostatic_prognostic_fields should return also the barotropic velocities when using a split-explicit free surface.

(3). I would like to flatten out the design of the SplitExplicitFreeSurface by

• removing the SplitExplicitState, and have the barotropic velocities and the mean fields part of the main free_surface type and all the remaining fields required for specific timesteppers in the timestepper which at the moment is very simple but can be redesigned to contain all time-stepping specific information
• removing SplitExplicitSettings (I don't know why there is a settings_kwargs there but it looks odd and it's probably a sign that this type is not well designed)

This leads to a SplitExplicitFreeSurface which will look like

struct SplitExplicitFreeSurface{H, U, M, FT, K , S, T} <: AbstractFreeSurface{H, FT}
    η :: H
    barotropic_velocities :: U # A namedtuple with U, V 
    filtered_state :: M # A namedtuple with η, U, V averaged throughout the substepping
    gravitational_acceleration :: FT
    kernel_parameters :: K
    substepping :: S  # Either `FixedSubstepNumber` or `FixedTimeStepSize`
    timestepper :: T # redesigned to contain all auxiliary field and settings necessary to the particular timestepping
end

These changes will not affect the performance nor the functioning of the algorithm, and (if there are none required) should not even affect the user interface.

[glwagner]

What about the topologically aware operators PR? Should that come first? That seems to me to be the most significant improvement.

Note there are some changes on #3867 to respect.

[glwagner]

removing SplitExplicitSettings (I don't know why there is a settings_kwargs there but it looks odd and it's probably a sign that this type is not well designed)

agree!

This leads to a SplitExplicitFreeSurface which will look like

This looks nice.

I don't think a new module is necessary but it could be done. The NonhydrostaticModel is a bit cleaner because many of the solvers have been abstract as "Poisson solvers", but the hydrostatic model does not benefit from this.

[simone-silvestri]

What about the topologically aware operators PR? Should that come first? That seems to me to be the most significant improvement.

Note there are some changes on #3867 to respect.

Right, I guess we can first #3268, then #3841 and then we can refactor the split explicit code

[ali-ramadhan]

The Topologically aware operators PR #3268 is ready for review and should be ready to merge I believe!