Integrate LES functionality #309

glwagner · 2019-07-03T19:32:28Z

This PR starts the processes of integrating LES functionality from glwagner/pass-tests into Oceananigans master. It is very much WIP at the moment.

…Closures

glwagner · 2019-07-05T19:03:56Z

@ali-ramadhan this PR is ready for a first review.

Possibly we should also add docs in this PR. I could also envision a few more tests to ensure the correctness of the turbulent diffusivities in simple flow fields for Constant Smagorinsky and Anisotropic Minimum Dissipation. However this last is not necessary.

ali-ramadhan · 2019-07-06T20:35:44Z

When we switch to finite volume operators (PR #283) it'll become much clearer where we are imposing no-flux and zero gradient conditions which should help along with comments like # no penetration and # no-gradient condition.

We also switch from ▶ to ϊ (\iota\ddot) for interpolation but we can take care of these later.

Would be good to have the turbulence closures using Oceananigans.Operators instead of redefining their own operators when I get back to working on #283 and #47.

ali-ramadhan · 2019-07-06T20:37:44Z

Possibly we should also add docs in this PR.

How much of it is similar with dedaLES documentation of turbulence closures? Might be mostly copy paste?

ali-ramadhan · 2019-07-06T20:42:33Z

Might be good to close #120 and release v0.9.0 once this PR is merged? Well, maybe release a new minor version once LES is verified.

ali-ramadhan

Looks good to me so far. With a minor fix all the tests pass (+ GPU tests on Engaging).

Aim of this PR should be to integrate LES closures ASAP and then we can worry about verifying LES and cleaning things up in future PRs.

ali-ramadhan · 2019-07-06T21:13:32Z

I could also envision a few more tests to ensure the correctness of the turbulent diffusivities in simple flow fields for Constant Smagorinsky and Anisotropic Minimum Dissipation. However this last is not necessary.

Hmmm, wonder if it's worth doing this if we're going to do some more rigorous LES verification anyways? The rigorous tests probably won't pass initially which will lead us to do these simpler tests first maybe.

codecov · 2019-07-06T22:35:05Z

Codecov Report

Merging #309 into master will decrease coverage by 6.39%.
The diff coverage is 63.63%.

@@            Coverage Diff            @@
##           master     #309     +/-   ##
=========================================
- Coverage   72.57%   66.17%   -6.4%     
=========================================
  Files          24       25      +1     
  Lines        1032     1159    +127     
=========================================
+ Hits          749      767     +18     
- Misses        283      392    +109

Impacted Files	Coverage Δ
src/Oceananigans.jl	`62.5% <ø> (-37.5%)`	⬇️
src/time_steppers.jl	`74.17% <100%> (-7.14%)`	⬇️
src/equation_of_state.jl	`100% <100%> (ø)`	⬆️
src/closures/constant_diffusivity_closures.jl	`65.62% <100%> (+2.29%)`	⬆️
src/closures/turbulence_closures.jl	`76.66% <22.22%> (-18.79%)`	⬇️
src/closures/closure_operators.jl	`51.63% <40.54%> (-6.81%)`	⬇️
src/closures/constant_smagorinsky.jl	`52.77% <51.61%> (+34.59%)`	⬆️
src/closures/anisotropic_minimum_dissipation.jl	`96.96% <96.96%> (ø)`
src/poisson_solvers.jl	`39.16% <0%> (-60.01%)`	⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fdff75...11e3043. Read the comment docs.

codecov · 2019-07-06T22:35:05Z

Codecov Report

Merging #309 into master will decrease coverage by 6.39%.
The diff coverage is 63.63%.

@@            Coverage Diff            @@
##           master     #309     +/-   ##
=========================================
- Coverage   72.57%   66.17%   -6.4%     
=========================================
  Files          24       25      +1     
  Lines        1032     1159    +127     
=========================================
+ Hits          749      767     +18     
- Misses        283      392    +109

Impacted Files	Coverage Δ
src/Oceananigans.jl	`62.5% <ø> (-37.5%)`	⬇️
src/time_steppers.jl	`74.17% <100%> (-7.14%)`	⬇️
src/equation_of_state.jl	`100% <100%> (ø)`	⬆️
src/closures/constant_diffusivity_closures.jl	`65.62% <100%> (+2.29%)`	⬆️
src/closures/turbulence_closures.jl	`76.66% <22.22%> (-18.79%)`	⬇️
src/closures/closure_operators.jl	`51.63% <40.54%> (-6.81%)`	⬇️
src/closures/constant_smagorinsky.jl	`52.77% <51.61%> (+34.59%)`	⬆️
src/closures/anisotropic_minimum_dissipation.jl	`96.96% <96.96%> (ø)`
src/poisson_solvers.jl	`39.16% <0%> (-60.01%)`	⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fdff75...11e3043. Read the comment docs.

christophernhill · 2019-07-08T20:47:42Z

@ali-ramadhan and @glwagner is the incmod(), decmod() stuff the right thing to do in this. I am trying to think how this will interact with Halo() code elsewhere? Would we be better off filling halos at various intermediate stages for now. We can then tidy up and use wiser halos later? Any thoughts?

ali-ramadhan · 2019-07-08T22:26:12Z

@ali-ramadhan and @glwagner is the incmod(), decmod() stuff the right thing to do in this. I am trying to think how this will interact with Halo() code elsewhere? Would we be better off filling halos at various intermediate stages for now. We can then tidy up and use wiser halos later? Any thoughts?

Not sure if this is the final form of this PR, but I suggested we just merge the closures as is, with operators that use incmod1 and decmod1, mostly to integrate the closures ASAP. It's definitely inconsistent with the rest of the fields that actually use halos.

We could focus on cleanup of this kind once we verify that the LES closures work, unless this is a pretty urgent issue.

christophernhill · 2019-07-08T22:53:35Z

@ali-ramadhan OK. I guess I am missing whether the LES can work with incmod at same time as structures are haloed? e.g in below, unless u is some sort of view (?), it looks slightly wrong to me if u[1] (for example) is in a halo?

@inline function ∂x_caa(i, j, k, grid, u::AbstractArray)
    i⁺ = incmod1(i, grid.Nx)
    return @inbounds (u[i⁺, j, k] - u[i, j, k]) / grid.Δx
end

ali-ramadhan · 2019-07-08T23:04:44Z

I think they should as the incmod1 operators should do the exact same job as the halos (assuming periodic boundary conditions), but might also be missing something.

glwagner · 2019-07-09T10:21:35Z

How much of it is similar with dedaLES documentation of turbulence closures? Might be mostly copy paste?

Math is the same, but docs syntax is different.

glwagner · 2019-07-09T10:25:36Z

@christophernhill @ali-ramadhan using incmod1 will probably work for periodic domains, as it is equivalent to ignoring the existence of halos.

Using incmod1 does defeat the work done to implement side wall boundary conditions.

christophernhill · 2019-07-09T10:28:39Z

Ok - got it!

…

On Tue, Jul 9, 2019 at 06:26 Gregory L. Wagner ***@***.***> wrote: @christophernhill <https://github.com/christophernhill> @ali-ramadhan <https://github.com/ali-ramadhan> using incmod1 will probably work for periodic domains, as it is equivalent to ignoring the existence of halos. Using incmod1 does defeat the work done to implement side wall boundary conditions. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#309>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA27DYASSNEZWAOQ7TFQKHLP6RRSDANCNFSM4H5I6LHA> .

ali-ramadhan · 2019-07-10T12:48:33Z

@glwagner From yesterday's meeting it sounds like this is ready to be merged?

glwagner · 2019-07-10T20:46:25Z

@ali-ramadhan I think so! Docs will come in a subsequent PR.

[WIP] Integrate LES functionality Former-commit-id: c961d39

glwagner added 12 commits July 3, 2019 15:22

updates turbulence closures and constant smag files

fdf2abe

adds new operators for precomputed diffusivities

daa1eb4

adds anisotropic minimum dissipation functionality

3fb551c

adds calc diffusivities functions

7972585

replaces closure operators with version using incmod1 and decmod1

e814b2d

doc strings update for constant smagorinsky

1f8d1e2

import amd

e72cf71

adds buoyancy to equation of state and uses salinity

a047719

exports buoyancy

181ce5d

adds function to instatantiate TurbulentDiffusivities

4f2ff09

exports TurbulentDiffusivities and calc_diffusivities from Turbulence…

4930d1e

…Closures

adds test for calc_diffusivities

1d3b0c3

glwagner requested review from ali-ramadhan and jm-c July 5, 2019 19:04

Add salinity S as argument to δρ. Resolves CliMA#20.

11e3043

ali-ramadhan added the feature 🌟 Something new and shiny label Jul 6, 2019

ali-ramadhan added this to the LES closures milestone Jul 6, 2019

ali-ramadhan approved these changes Jul 6, 2019

View reviewed changes

ali-ramadhan mentioned this pull request Jul 10, 2019

Stratified plane Couette flow test #310

Closed

ali-ramadhan merged commit c961d39 into CliMA:master Jul 11, 2019

glwagner deleted the integrate-les branch July 24, 2019 19:57

ali-ramadhan changed the title ~~[WIP] Integrate LES functionality~~ Integrate LES functionality Aug 1, 2019

arcavaliere pushed a commit to arcavaliere/Oceananigans.jl that referenced this pull request Nov 6, 2019

Merge pull request CliMA#309 from glwagner/integrate-les

3c91493

[WIP] Integrate LES functionality Former-commit-id: c961d39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate LES functionality #309

Integrate LES functionality #309

glwagner commented Jul 3, 2019

glwagner commented Jul 5, 2019

ali-ramadhan commented Jul 6, 2019 •

edited

Loading

ali-ramadhan commented Jul 6, 2019

ali-ramadhan commented Jul 6, 2019

ali-ramadhan left a comment

ali-ramadhan commented Jul 6, 2019

codecov bot commented Jul 6, 2019

codecov bot commented Jul 6, 2019

christophernhill commented Jul 8, 2019

ali-ramadhan commented Jul 8, 2019 •

edited

Loading

christophernhill commented Jul 8, 2019

ali-ramadhan commented Jul 8, 2019

glwagner commented Jul 9, 2019

glwagner commented Jul 9, 2019

christophernhill commented Jul 9, 2019 via email

ali-ramadhan commented Jul 10, 2019 •

edited

Loading

glwagner commented Jul 10, 2019

Integrate LES functionality #309

Integrate LES functionality #309

Conversation

glwagner commented Jul 3, 2019

glwagner commented Jul 5, 2019

ali-ramadhan commented Jul 6, 2019 • edited Loading

ali-ramadhan commented Jul 6, 2019

ali-ramadhan commented Jul 6, 2019

ali-ramadhan left a comment

Choose a reason for hiding this comment

ali-ramadhan commented Jul 6, 2019

codecov bot commented Jul 6, 2019

Codecov Report

codecov bot commented Jul 6, 2019

Codecov Report

christophernhill commented Jul 8, 2019

ali-ramadhan commented Jul 8, 2019 • edited Loading

christophernhill commented Jul 8, 2019

ali-ramadhan commented Jul 8, 2019

glwagner commented Jul 9, 2019

glwagner commented Jul 9, 2019

christophernhill commented Jul 9, 2019 via email

ali-ramadhan commented Jul 10, 2019 • edited Loading

glwagner commented Jul 10, 2019

ali-ramadhan commented Jul 6, 2019 •

edited

Loading

ali-ramadhan commented Jul 8, 2019 •

edited

Loading

ali-ramadhan commented Jul 10, 2019 •

edited

Loading