search

CliMA / ClimaOcean.jl

🌎 Tools for realistic regional-to-global ocean simulations, and coupled ocean + sea-ice simulations based on Oceananigans and ClimaSeaIce. Basis for the ocean and sea-ice component of CliMA's Earth system model.
https://clima.github.io/ClimaOceanDocumentation/dev/
MIT License
26 stars 7 forks source link

Why don't we use the constructor `WENOVectorInvariant` in `ocean_simulation`? #129

Open glwagner opened 1 month ago

glwagner commented 1 month ago

The constructor was created to facilitate construction of advection schemes for hydrostatic models that use WENO. However, it's not being used.

https://github.com/CliMA/ClimaOcean.jl/blob/9005d74a63668ca85863579091e23eaf956c3f91/src/OceanSimulations/OceanSimulations.jl#L31-L33

We should:

  1. Use it here, since that's its purpose.
  2. If it can't be used here, change it so that it can be used.
  3. If its not useful, delete it.
glwagner commented 1 month ago

Also the default vertical_scheme is EnergyConserving(), but here we use Centered():

VectorInvariant
search: VectorInvariant VectorInvariantFormulation WENOVectorInvariant

  VectorInvariant(; vorticity_scheme = EnstrophyConserving(),
                    vorticity_stencil = VelocityStencil(),
                    vertical_scheme = EnergyConserving(),
                    kinetic_energy_gradient_scheme = vertical_scheme,
                    divergence_scheme = vertical_scheme,
                    upwinding = OnlySelfUpwinding(; cross_scheme = vertical_scheme),
                    multi_dimensional_stencil = false)

  Return a vector invariant momentum advection scheme.

  Keyword arguments
  ≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡

    •  vorticity_scheme: Scheme used for Center reconstruction of vorticity. Default: EnstrophyConserving(). Options:
       • UpwindBiased()
       • WENO()
       • EnergyConserving()
       • EnstrophyConserving()

    •  vorticity_stencil: Stencil used for smoothness indicators for WENO schemes. Default: VelocityStencil(). Options:
       • VelocityStencil() (smoothness based on horizontal velocities)
       • DefaultStencil() (smoothness based on variable being reconstructed)

    •  vertical_scheme: Scheme used for vertical advection of horizontal momentum. Default: EnergyConserving().

why haven't we changed the default?

simone-silvestri commented 1 month ago

Because the default was not working due to a bug when this code was written. I fixed that bug so you can change this default.

glwagner commented 1 month ago

Even if the bug was fixed, the docstring for WENOVectorInvariant doesn't make it clear how to build the advection scheme above:

help?> WENOVectorInvariant
search: WENOVectorInvariant

  WENOVectorInvariant(; upwinding = nothing,
                        multi_dimensional_stencil = false,
                        weno_kw...)

the docstring also needs to be updated in Oceananigans, in addition to invoking it here.