CliMA / Oceananigans.jl

🌊 Julia software for fast, friendly, flexible, ocean-flavored fluid dynamics on CPUs and GPUs
https://clima.github.io/OceananigansDocumentation/stable
MIT License
958 stars 191 forks source link

Cleaning up TurbulenceClosures (especially those for large eddy simulation) #1381

Closed glwagner closed 2 years ago

glwagner commented 3 years ago

A number of issues are accumulating with TurbulenceClosures that we need to address:

  1. 1277 (inconsistency between docs and storage of molecular diffusivity in eddy viscosity scratch array)

  2. 1278 (unfortunate default diffusivities for IsotropicDiffusivity and some of the LES closure diffusivities)

  3. 1279 (incorrect usage of the term "biharmonic", and also the fact that there is no true biharmonic diffusivity)

A second annoying issue is the use of model.diffusivities as auxiliary space for the eddy diffusivities. Now that we know a bit more about using adapt_structure, we can readily put references to eddy diffusivities inside the LES closure objects themselves. Doing this will require refactoring the constructor for IncompressibleModel, unfortunately, because we'll need to define a function regularize_turbulence_closure so that we can build scratch space once we know what the grid is.

I also think it's high time that we nuked RozemaAnisotropicMinimumDissipation and BlasiusSmagorinsky, and also eliminated the "buoyancy modification" term from VerstappenAnisotropicMinimumDissipation, since it doesn't actually work --- and because we have reason to believe that it may not actually improve the accuracy of the closure, even if correctly implemented.

Lastly, I think much of the code in TurbulenceClosures could be consolidated.

I wanted to put this issue out there in case anyone is somehow using the Rozema version of AMD or Blasius version of Smagorinsky that I don't know about.

These are not our most urgent priorities now, but after peeking into the turbulence closures module recently I think it'll be useful to gather all the various improvements we need into one issue.

ali-ramadhan commented 3 years ago

Since this is a meta-issue about TurbulenceClosures gripes, I'll copy my list of notational changes I thought would be nice from #654 (some might have already been addressed...):

  1. Rename ∂ⱼ_2ν_Σ₁ⱼ to ∂ⱼτ₁ⱼ as τᵢⱼ = 2νSᵢⱼ is would no longer be true for CompressibleModel, it's τᵢⱼ = 2νSᵢⱼ + λSₘₘδᵢⱼ where λ is the second viscosity, or τᵢⱼ = 2μ(Sᵢⱼ - ⅓Sₖₖδᵢⱼ) + μᵥSₖₖδᵢⱼ where μᵥ is the bulk or volume viscosity.

  2. For constant isotropic viscosity: rename ∂ⱼνᵢⱼ∂ᵢu to ν∇²u. For constant isotropic diffusivity: rename ∂ⱼκᵢⱼ∂ᵢc to κ∇²c.

  3. For constant anistropic viscosity: rename ∂ⱼνᵢⱼ∂ᵢu to νⱼ∂ⱼ²u. For constant anistropic diffusivity: rename ∂ⱼκᵢⱼ∂ᵢc to κⱼ∂ⱼ²c. Potentially a misuse of summation notation but should be a valid and understandable use of it.

  4. For ConstantAnisotropicDiffusivity and AnisotropicBiharmonicDiffusivity should we generalize it to be truly anisotropic with three diagonal components?

  5. Use Sᵢⱼ instead of Σᵢⱼ to denote the strain-rate tensor, following a common convention in most books, papers, and Wikipedia. I believe Σ was used initially to not conflict with salinity S but I think the use of salinity has is now mainly restricted to Oceananigans.Buoyancy so there is a very small chance for confusion in Oceananigans.TurbulenceClosures.

glwagner commented 3 years ago

Ah nice!

∂ⱼτ₁ⱼ is a great change also because this is also more generally valid for closures that do not have eddy diffusivities (which we will add very soon).

Potentially a misuse of summation notation but should be a valid and understandable use of it.

That looks ok to me.

For ConstantAnisotropicDiffusivity and AnisotropicBiharmonicDiffusivity should we generalize it to be truly anisotropic with three diagonal components?

Note that this is already the case for AnisotropicDiffusivity. For fourth-order dissipation I propose using AnisotropicFourthOrderDiffusivity or AnisotropicHyperDiffusivity with general diagonal components, and adding another AnisotropicBiharmonicDiffusivity that is actually biharmonic in the horizontal (the biharmonic operator is the Laplacian squared, rather than dx^4 + dy^4).

Use Sᵢⱼ instead of Σᵢⱼ to denote the strain-rate tensor

We can do this. But I do know plenty of papers that use Σᵢⱼ 😢

ali-ramadhan commented 3 years ago

Ah haha Sᵢⱼ vs. Σᵢⱼ can certainly be opinionated. I have no skin in this game.

glwagner commented 3 years ago

1002 is related.