Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Outdated docs on background diffusivity/viscosity #3649

Open
whitleyv opened this issue Jul 8, 2024 · 1 comment · May be fixed by #3673
Open

Outdated docs on background diffusivity/viscosity #3649

whitleyv opened this issue Jul 8, 2024 · 1 comment · May be fixed by #3673
Labels
cleanup 🧹 Paying off technical debt documentation 📜 The sacred scrolls

Comments

@whitleyv
Copy link
Collaborator

whitleyv commented Jul 8, 2024

Some of the docs still include the background viscosity in the description of Smagorinsky-Lilly. Maybe you're purposefully keeping the "Physics" different than the "Model," but if it's a typo, these are the two places it's the old version:

The eddy viscosity is given by
```math
\begin{align}
\nu_e = \left ( C \Delta_f \right )^2 \sqrt{ \Sigma^2 } \, \varsigma(N^2 / \Sigma^2) + \nu \, ,
\label{eq:smagorinsky-viscosity}
\end{align}
```

where the eddy diffusivity ``\kappa_e`` is
```math
\kappa_e = \frac{\nu_e - \nu}{Pr} + \kappa \, ,
```
where ``\kappa`` is a constant isotropic background diffusivity.
Both ``Pr`` and ``\kappa`` may be set independently for each tracer.

This description in the model setup is also a bit misleading, or, at least, unclear.

although they may be specified. By default, the background viscosity and diffusivity are assumed to
be the molecular values for seawater. For more details see [`SmagorinskyLilly`](@ref).

@whitleyv whitleyv added cleanup 🧹 Paying off technical debt documentation 📜 The sacred scrolls labels Jul 8, 2024
@glwagner
Copy link
Member

glwagner commented Jul 9, 2024

Maybe you're purposefully keeping the "Physics" different than the "Model,"

I don't think so!

Previously we made the argument that the background viscosity is not part of the "eddy viscosity", conceptually. And of course as you point out that's not how its implemented currently either. So I agree this should be fixed.

This description in the model setup is also a bit misleading, or, at least, unclear.

Totally, this is wrong, there is no background viscosity of diffusivity when using SmagorinskyLilly either settable or by default.

@glwagner glwagner linked a pull request Jul 30, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Paying off technical debt documentation 📜 The sacred scrolls
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants