Improvements to stretched `WENO5` API and new method `validate_advection`

[glwagner]

A few ideas:

1. Allow grid as a positional argument so we can write WENO5(grid) rather than WENO5(grid=grid)
2. Emit a warning about "stretched WENO" in the model constructor rather than WENO5. It's misleading to throw a warning when using WENO5 on a uniform grid; we should only throw a warning if users specify WENO5 with a stretched grid.
3. Alternatively to 2, we can re-build WENO in model constructors with a validate_advection method. We already have validate_momentum_advection for hydrostatic models:

https://github.com/CliMA/Oceananigans.jl/blob/b3ddbc84c8f35aaf5b93fbbfdb4cffcada5c6533/src/Models/HydrostaticFreeSurfaceModels/hydrostatic_free_surface_model.jl#L127

The downside to 3 is that it's bad for testing, since it might prevent us from using the "uniform" flavor of WENO on stretched grids. (@simone-silvestri is that true?) Also something that's not clear to me --- @simone-silvestri do coefficients like

https://github.com/CliMA/Oceananigans.jl/blob/b3ddbc84c8f35aaf5b93fbbfdb4cffcada5c6533/src/Advection/weno_fifth_order.jl#L30

become Nothing in regular directions, even with WENO5(grid=grid)? Or are they only Nothing for WENO5()?

[glwagner]

Couple random notes and questions:

https://github.com/CliMA/Oceananigans.jl/blob/b3ddbc84c8f35aaf5b93fbbfdb4cffcada5c6533/src/Advection/weno_fifth_order.jl#L177-L178

belong in Grids.

Why W isa Number for zweno == true?

https://github.com/CliMA/Oceananigans.jl/blob/b3ddbc84c8f35aaf5b93fbbfdb4cffcada5c6533/src/Advection/weno_fifth_order.jl#L171

That line should be written

W = zweno ? Number : Nothing

but note we can also simply use zweno as the type parameter directly so

const JSWENO = WENO5{<:Any, <:Any, <:Any, <:Any, <:Any, <:Any, <:Any, false}
const ZWENO  = WENO5{<:Any, <:Any, <:Any, <:Any, <:Any, <:Any, <:Any, true}

might make senes to move that parameter to the top of the struct.

The weight calculations in the code are difficult to read and understand:

https://github.com/CliMA/Oceananigans.jl/blob/b3ddbc84c8f35aaf5b93fbbfdb4cffcada5c6533/src/Advection/weno_fifth_order.jl#L642-L668

How can we improve this? I think more describe names would help self-document at least (eg what are j, c, m, num, l, i, op?). We probably should add a specific reference to an equation in the comment / docstring.

[glwagner]

It looks like validate_advection is also needed if we want to support a nice API for changing the floating point precision:

https://github.com/CliMA/Oceananigans.jl/blob/b3ddbc84c8f35aaf5b93fbbfdb4cffcada5c6533/src/Advection/weno_fifth_order.jl#L139

Right now users have to specify Float32 in both the grid and WENO5 to get reduced-precision all around.

If we validate_advection we can use the "stub, regularization" strategy where FT is set to eltype(grid) only if it's not specificed (to support existing behavior where FT can be set indepednently of eltype(grid).)

[simone-silvestri]

Agreed with the suggestions!

Indeed, coefficients are nothing in unstretched directions

@inline calc_interpolating_coefficients(FT, coord::OffsetArray{<:Any, <:Any, <:AbstractRange}, arch, N) = nothing
@inline calc_interpolating_coefficients(FT, coord::AbstractRange, arch, N)  = nothing

[glwagner]

Yay!

Ok, then we can just use a special type like struct Default end to denote that WENO() has not been constructed with a grid. That will allow us (or advanced users) to use an "unstretched" WENO on a stretched grid if testing / some emergency requires it. Otherwise --- if we think it's wise --- we'll just always use the stretched version on a stretched grid.

[tomchor]

Didn't https://github.com/CliMA/Oceananigans.jl/pull/2317 solve this issue?

[simone-silvestri]

I did some partial cleanup in #2317, but that PR was more about introducing a new advection scheme. There is still a bit of cleanup to be done actually

[glwagner]

Let's called it closed