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.

Sign up for GitHub

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

Jump to bottom

TKEDissipationVerticalDiffusivity (aka k-epsilon) turbulence closure (take 2) #3605

Merged
merged 80 commits into from
Jun 20, 2024
Merged

TKEDissipationVerticalDiffusivity (aka k-epsilon) turbulence closure (take 2) #3605

merged 80 commits into from
Jun 20, 2024

Conversation

glwagner
Copy link
Member

This PR replaces #3509. Also, it builds off #3585 which hopefully we can merge soon so we don't enter merge conflict hell.

In short this PR implements k-epsilon. Although, the version that's implemented now is not by-the-book k-epsilon but rather a simplified version. I'd like to merge this PR basically as is and hopefully continue to work on it in the future.

There are some numerical considerations which should be reviewed and documented I think. First of all is the challenge of specifying a minimum tke, minimum dissipation rate, and also a maximum dissipation rate. These are useful for producing smooth diffusivities, which note are

$$ \kappa \sim \frac{e^2}{\epsilon} $$

But then there are challenges with implementing the TKE and dissipation equations. Because depending on the term, one may either want to impose or not impose these constraints.

It could be useful to try to get some lessons from other codes too.

It's not ready for production use, but at least it's doing reasonable things...

image

@glwagner
Generalize definition of Ri
40033f2
@glwagner
Non-negative Ri
af70671
@glwagner
Use quasi-conservative reconstruction of shear production and buoyanc…
ba01fce 
…y flux
@glwagner
Add convective mixing length for momentum
06fbe39
@glwagner
Update minimum TKE
6e28dce
@glwagner
Merge remote-tracking branch 'origin/main' into glw/convective-moment…
cc44aa7 
…um-mixing
@glwagner
Major reorganizations to get CATKE time-stepping to work
f3ca3d7
@glwagner
Working on comparison
a6c6921
@glwagner
Some updates
af67213
@glwagner
Heroics to get things to wokrk
871654e
@glwagner
Update tke time stepping
bd90a86
@glwagner
Bit of cleanup
a163077
@glwagner
Figure out the right temporal stencil
24012b1
@glwagner
Updated default convective buoyancy flux and delete ccc methods
38e0686
@glwagner
Add back convective mixing length for ccc
44f2b84
@glwagner
Grr also add back stability function for ccc
797445a
@glwagner
Better error message when setting fails for field time series
86a6621
@glwagner
Better notation for surface buoyancy flux;
511fdbe
@glwagner
Implement split-explicit time-stepping for CATKE
8743eca
@glwagner
Optimize CATKE substepping a bit more
841224f
@glwagner
Performance enhancements
5e15e18
@glwagner
Add third method to CATKE comparison
9705517
@glwagner
Minor improvements
3ba9dc2
@glwagner
Clean up
f0f4283
@glwagner
Update
d989c4e
@glwagner
Move around some inbounds
7aaf4a7
@glwagner
Merge branch 'main' into glw/split-explicit-catke
287d2ef
@glwagner
Delete unused code
7bf3d47
@glwagner
Clean up time stepping
4f671f2
@glwagner
Little things
2c432d7
@glwagner glwagner requested a review from navidcy June 7, 2024 23:13
@navidcy navidcy added turbulence closures 🎐 parameterizations Fudge factors and hand-waving labels Jun 10, 2024
@navidcy
Copy link
Collaborator

navidcy commented Jun 10, 2024

OK! I'll review this!

so in Oceananigans terms is the e-ϵ parameterization, right?

navidcy
navidcy approved these changes Jun 10, 2024
View reviewed changes
Copy link
Collaborator

@navidcy navidcy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job; sprinkled few questions/comments here and there

...lence_closure_implementations/TKEBasedVerticalDiffusivities/TKEBasedVerticalDiffusivities.jl Outdated Show resolved Hide resolved
...rbulence_closure_implementations/TKEBasedVerticalDiffusivities/catke_vertical_diffusivity.jl Outdated Show resolved Hide resolved
...losure_implementations/TKEBasedVerticalDiffusivities/tke_dissipation_vertical_diffusivity.jl Outdated Show resolved Hide resolved
...losure_implementations/TKEBasedVerticalDiffusivities/tke_dissipation_vertical_diffusivity.jl Outdated Show resolved Hide resolved
...losure_implementations/TKEBasedVerticalDiffusivities/tke_dissipation_vertical_diffusivity.jl Outdated Show resolved Hide resolved
...urbulence_closure_implementations/TKEBasedVerticalDiffusivities/tke_dissipation_equations.jl Outdated Show resolved Hide resolved
...urbulence_closure_implementations/TKEBasedVerticalDiffusivities/tke_dissipation_equations.jl Outdated Show resolved Hide resolved
...urbulence_closure_implementations/TKEBasedVerticalDiffusivities/tke_dissipation_equations.jl Outdated Show resolved Hide resolved
...closure_implementations/TKEBasedVerticalDiffusivities/tke_dissipation_stability_functions.jl Show resolved Hide resolved
...losure_implementations/TKEBasedVerticalDiffusivities/tke_dissipation_vertical_diffusivity.jl Outdated Show resolved Hide resolved
navidcy and others added 9 commits June 12, 2024 14:48
@navidcy @simone-silvestri
fix typo
a544911 
Co-authored-by: Simone Silvestri <[email protected]>
@navidcy
Merge branch 'main' into glw/k-epsilon-2
2986177
@glwagner @navidcy
Update src/TurbulenceClosures/turbulence_closure_implementations/TKEB…
ac0de46 
…asedVerticalDiffusivities/catke_vertical_diffusivity.jl

Co-authored-by: Navid C. Constantinou <[email protected]>
@glwagner @navidcy
Update src/TurbulenceClosures/turbulence_closure_implementations/TKEB…
9abdcfe 
…asedVerticalDiffusivities/tke_dissipation_equations.jl

Co-authored-by: Navid C. Constantinou <[email protected]>
@glwagner
Restore convective adjustment notation
096467a
@glwagner
Address a few review comments
b15fecf
@glwagner
Get rid of background diffusivities
2397ebe
@glwagner @navidcy
Update src/TurbulenceClosures/turbulence_closure_implementations/TKEB…
6d8f883 
…asedVerticalDiffusivities/tke_dissipation_vertical_diffusivity.jl

Co-authored-by: Navid C. Constantinou <[email protected]>
@glwagner
Fix docstring
7394dda
@glwagner
Copy link
Member Author

OK! I'll review this!

so in Oceananigans terms is the e-ϵ parameterization, right?

I think we should call it "TKE dissipation vertical diffusivity", or k-epsilon in a nod to the literature. I don't think we shoudl call it "e-epsilon".

@navidcy
Copy link
Collaborator

navidcy commented Jun 13, 2024 •
edited

I think we should call it "TKE dissipation vertical diffusivity", or k-epsilon in a nod to the literature. I don't think we shoudl call it "e-epsilon".

No, definitely not!! I wasn't implying that we call it "e-epsilon". I was just trying to see whether the k from k-ε is the same as our e... But after my question I soon figured it out -- sorry, I should have deleted the comment.

Let's call it "TKE dissipation vertical diffusivity" but let's also mention in the docstring or somewhere that this is "the otherwise known as k-epsilon"

@glwagner
Copy link
Member Author

I think we should call it "TKE dissipation vertical diffusivity", or k-epsilon in a nod to the literature. I don't think we shoudl call it "e-epsilon".

No, definitely not!! I wasn't implying that we call it "e-epsilon". I was just trying to see whether the k from k-ε is the same as our e... But after my question I soon figured it out -- sorry, I should have deleted the comment.

Let's call it "TKE dissipation vertical diffusivity" but let's also mention in the docstring or somewhere that this is "the otherwise known as k-epsilon"

Ah yes, "k" is turbulent kinetic energy in the name "k-epsilon".

@glwagner glwagner merged commit 5661358 into main Jun 20, 2024
46 checks passed
@glwagner glwagner deleted the glw/k-epsilon-2 branch June 20, 2024 22:09
glwagner added a commit that referenced this pull request Jun 20, 2024
@glwagner
Bump to 0.91.3
cc8d6f5 
Captures #3605
@glwagner glwagner mentioned this pull request Jun 20, 2024
Merged
glwagner added a commit that referenced this pull request Jun 21, 2024
@glwagner
(0.91.3) Bump (#3633)
d5ef2a4 
* Bump to 0.91.3

Captures #3605

* Bump down

* Bump up
@glwagner glwagner mentioned this pull request Jun 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iuryt iuryt iuryt left review comments

@navidcy navidcy navidcy approved these changes

@simone-silvestri simone-silvestri simone-silvestri left review comments

Assignees
No one assigned
Labels
parameterizations Fudge factors and hand-waving turbulence closures 🎐
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@glwagner @navidcy @iuryt @simone-silvestri