Diagnostics module #508

kpamnany · 2019-10-30T15:38:14Z

Refactored implementation of the diagnostics prototyped in #467.

This introduces the first version of an infrastructure for gathering diagnostics on the DG grid and moves the diagnostics implemented in #467 onto that infrastructure. It also moves the code into a CLIMA module.

Diagnostics gathering is still triggered by the callback mechanism (and must be registered in the driver) until the proper timestepping mechanism is implemented.

Output is to JLD2 and VariableTemplates can be used with the help of the *_vars.jl files introduced here for ease in post-processing.

In future PRs:

More abstraction and integration with Oceananigans diagnostics
Reducing MPI communication

@slifer50 and @smarras79: please check the output for correctness relative to the previous PR.

Cc: @tapios, @sandreza, @christophernhill, @blallen, @glwagner

kpamnany · 2019-10-30T15:42:56Z

@jkozdon's comment that the "sums should be integral approximations (involving quadrature weights and jacobian determinants)" still applies. That is hopefully easier to implement now.

codecov · 2019-10-30T16:38:54Z

Codecov Report

Merging #508 into master will increase coverage by 3.13%.
The diff coverage is 83.17%.

@@            Coverage Diff             @@
##           master     #508      +/-   ##
==========================================
+ Coverage   68.02%   71.15%   +3.13%     
==========================================
  Files          82       83       +1     
  Lines        5620     5814     +194     
==========================================
+ Hits         3823     4137     +314     
+ Misses       1797     1677     -120

Impacted Files	Coverage Δ
src/CLIMA.jl	`100% <ø> (ø)`	⬆️
src/Diagnostics/graph_diagnostic.jl	`0% <0%> (ø)`
src/Diagnostics/diagnostic_vars.jl	`100% <100%> (ø)`
...c/Utilities/VariableTemplates/VariableTemplates.jl	`94.87% <80%> (+0.08%)`	⬆️
src/Diagnostics/Diagnostics.jl	`91.52% <91.52%> (ø)`
src/DGmethods/DGmodel_kernels.jl	`97.14% <0%> (+0.19%)`	⬆️
src/Arrays/MPIStateArrays.jl	`68.87% <0%> (+6.12%)`	⬆️
... and 8 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 a1bc4b2...d0c81ec. Read the comment docs.

smarras79 · 2019-10-30T16:57:59Z

@slifer50 and @smarras79: please check the output for correctness relative to the previous PR.

will try it between later today and tomorrow and let you. Thanks for making this a module!

tapios · 2019-11-05T01:06:08Z

examples/DGmethods/ex_001_dycoms.jl

+  H     = Rm_sfc * T_sfc / grav;
+  p     = P_sfc * exp(-xvert/H);
+  #Density, Temperature
+  TS    = LiquidIcePotTempSHumEquil_no_ρ(θ_liq, q_pt, p)


This will need to be updated after PR #506 is merged. @charleskawczynski can help.

Is it possible to get density here and pass it in? Or do we need a LiquidIcePotTempSHumEquil_given_pressure(θ_liq, q_pt, p) constructor? I tried looking into this recently and ran into some problems with the equations.

If you can get T from theta_l and q_t using your functions then you can get rho from the equation of state using p and T as I do below in my matlab script (@tapios please, confirm that the operations below would be correct here):

`p = p_sfc * exp(-z / H) %Exner exner = (p / p_sfc)^(R_d/cp_d); %T, Tv T = exner*theta_liq + Lv0 * q_liq / (cpm*exner); %Density rho = p/(Rm*T); `

If you can get T from theta_l and q_t using your functions then you can get rho from the equation of state using p and T as I do below in my matlab script (@tapios please, confirm that the operations below would be correct here):

`p = p_sfc * exp(-z / H) %Exner exner = (p / p_sfc)^(R_d/cp_d);

This looks ok.

%T, Tv
T = exnertheta_liq + Lv0 * q_liq / (cpmexner);

The second term should not be divided by the Exner function: Just

T = exner*theta_liq + Lv0 * q_liq / (cpm);

(I put the derivations in the design docs; please refer to those.)

@charleskawczynski I think providing a function LiquidIcePotTempSHumEquil_given_pressure(θ_liq, q_pt, p) would be useful. In essence, this is how @smarras79 is calculating T.

%Density
rho = p/(Rm*T);
`

Just for reference, we'll have to use LiquidIcePotTempSHumNonEquil_given_pressure for now until we come up with a consistent constructor for an equilibrium phase.

This driver is now outdated and needs to be replaced with the currently running one.

smarras79 · 2019-11-05T01:42:30Z

Perfect. We speak tomorrow.

On Mon, Nov 4, 2019 at 8:06 PM Tapio Schneider ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In examples/DGmethods/ex_001_dycoms.jl <#508 (comment)>: > - #Temperature - T = exner_dry*θ_liq + LH_v0*q_liq/(cpm*exner_dry); - #Density - ρ = P/(Rm*T); - #Potential Temperature - θv = virtual_pottemp(T, P, q_pt) - # energy definitions - u, v, w = DT(7), DT(-5.5), DT(0) - U = ρ * u - V = ρ * v - W = ρ * w - e_kin = DT(1//2) * (u^2 + v^2 + w^2) + H = Rm_sfc * T_sfc / grav; + p = P_sfc * exp(-xvert/H); + #Density, Temperature + TS = LiquidIcePotTempSHumEquil_no_ρ(θ_liq, q_pt, p) This will need to be updated after PR #506 <#506> is merged. @charleskawczynski <https://github.com/charleskawczynski> can help. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#508>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALBNJHJ3SDKMVH23ZX22MULQSDBIFANCNFSM4JG3SJ3Q> .

-- Please, forgive the typos , I am writing from my phone!

charleskawczynski · 2019-11-05T16:36:02Z

src/Diagnostics/Diagnostics.jl

+
+    e_int = e̅_tot - 1//2 * (u̅^2 + v̅^2 + w̅^2) - grav * zvals[k,ev]
+
+    ts = PhaseEquil(convert(FT, e_int), q̅_tot, state.ρ)


Might be nice if we could pass in moist::MoistureModel, orientation::Orientation, state::Vars, aux::Vars and dispatch to the dry or moist thermo state functions instead, but maybe that'll be the next step.

Agree! We have state and can easily have aux. Would moist be this and orientation be this? Or would they need to come from somewhere else?

Yep, those are the correct structs.

src/Diagnostics/Diagnostics.jl

smarras79 · 2019-11-18T16:51:07Z

src/Diagnostics/Diagnostics.jl

+    u̅ = state.ρu[1] / state.ρ
+    v̅ = state.ρu[2] / state.ρ
+    w̅ = state.ρu[3] / state.ρ
+    q̅_tot = state.moisture.ρq_tot / state.ρ
+    ũ = ha.ρu / ha.ρ
+    ṽ = ha.ρv / ha.ρ
+    w̃ = ha.ρw / ha.ρ
+    ẽ = ha.e_tot / ha.ρ
+    q̃_tot = ha.ρq_tot / ha.ρ
+


I would suggest changing the notation to avoid confusion. The tilde and overbar look too similar and can lead to coding mistakes.
For example:
mean u --> \underline{u}
instantaneous u: --> u
fluctuation of u: --> \tilde{u}

or anything that clearly distinguishes each quantity

I don't have a preference -- your thoughts @akshaysridhar?

Agree with instantaneous u: --> u , this affects 185 to 188 and quantities dependent on these variables. Fluctuating quantities seem fine with \prime to me. No preference for the rest.

smarras79 · 2019-11-19T13:59:20Z

src/Diagnostics/diagnostic_vars.jl

+    z::FT
+    # state and functions of state
+    u::FT
+    v::FT
+    w::FT
+    q_tot::FT
+    e_tot::FT
+    q_liq::FT
+    θ::FT
+    θ_liq::FT
+    θ_v::FT
+    e_int::FT
+    # vertical fluxes
+    w′ρ′::FT
+    w′u′::FT
+    w′v′::FT
+    w′q_tot′::FT
+    w′q_liq′::FT
+    w′q_vap′::FT
+    w′θ′::FT
+    w′θ_v′::FT
+    w′θ_liq′::FT
+    # variances
+    u′u′::FT
+    v′v′::FT
+    w′w′::FT
+    # skewness
+    w′w′w′::FT


Although we use symbols within the main clima code, it would be better to not use symbols for the variables that are written to file.
Not every user may be able to use symbols when calling @kpamnany 's new graph_diagnostics.jl from the command line
(Notice the graph_diagnostics.jl is passed the list of variables to be plotted:
>> julia graph_diagnostics.jl <diagnostic_file.jld2> <diagnostic_name> where
is a list of the variables to be plotted.

In atmospheric software packages we usually find a combination of short and sometime full names such as:
"TH", "Potential temperature"
"THV", "Virtual potential temperature"
"PRS", "Pressure"
"rho", "Air density"
"qv", "Water vapor mixing ration"
etc. (these are examples from CM1 by Bryan, but they all more or less use a similar convention)
Bottom line, I would totally avoid greek symbols to name the diagnostic variables.

Suggestions (@kpamnany @akshaysridhar @tapios):

<w′θ′> --> vert_eddy_heat_flx
<wθ> --> vert_heat_flx
<w′q_tot>′ --> vert_eddy_qt_flx
<wq_tot> --> vert_qt_flx
<u′u′> --> uvariance
<u′u′> --> vvariance
<w′w′> --> wvariance
<w′u′> --> vert_eddy_u_flx
<w′u′> --> vert_eddy_v_flx

etc.

Added altvars_diagnostic, as a mirror of vars_diagnostic, with some of the changes suggested here. If you give me a full list of what you'd like changed, I'll make the changes and then update graph_diagnostic.jl to use altvars_diagnostic instead, which will allow you to use non-unicode names in post-processing while still allowing the CLIMA code to use unicode.

Not to make this go back and forth but, I think a better solution here would be to write mapping functions for each symbol/expression, and calls to print/show can be intercepted by the mapping funcs, instead of having two structs of the same thing.

Or use something like:

struct VerboseName{T} end VerboseName(T) = VerboseName{T}() nice_name(::VerboseName{:θ}) = :theta nice_name(::VerboseName{:e_int}) = :e_int struct Foo{FT} θ::FT e_int::FT end nice_names(f::Foo) = nice_name.(VerboseName.(fieldnames(typeof(f)))) f = Foo(1.0,2.0) @show nice_names(f)

Agreed. In the diagnostics issue, I outlined what we’ll need in the end: a table that defines short and verbose names for the output fields, and units, to be able to generate self-documenting output. Short names will be variable names in files. May not need to be now, but that’s where we’ll need to go eventually.

Having primes etc. in the code is fine for now though. I wouldn’t go through the trouble to create more conventional variable names in the code right now.

I'll just switch to the conventional variable names now, since we want to do that anyway.

I'll just switch to the conventional variable names now, since we want to do that anyway.

If by conventional you mean the long names @smarras79 suggested, my take is that is not needed right now. There are no general conventions for the names, and your primed notation had the advantage of clarity for now. Most importantly, it think it would be good to get this merged, and then we can refine.

Simone has been working with a modified version of this code with plain text names due to difficulties with specifying the Unicode names we're using. I've just switched to his version; not a difficult thing. I agree on getting this merged ASAP!

src/Diagnostics/graph_diagnostic.jl

src/Diagnostics/Diagnostics.jl

kpamnany · 2019-11-26T16:24:48Z

@slifer50 has written a test for this PR per the suggestion from @tapios but it doesn't work too well in the absence of #545. So we're waiting on that to be merged after which we will update this and also add the test.

kpamnany · 2019-12-16T14:36:37Z

@simonbyrne: this is good to go.

simonbyrne · 2019-12-16T18:15:15Z

bors r+

@slifer50

508: Diagnostics module r=simonbyrne a=kpamnany Refactored implementation of the diagnostics prototyped in #467. This introduces the first version of an infrastructure for gathering diagnostics on the DG grid and moves the diagnostics implemented in #467 onto that infrastructure. It also moves the code into a CLIMA module. Diagnostics gathering is still triggered by the callback mechanism (and must be registered in the driver) until the proper timestepping mechanism is implemented. Output is to JLD2 and `VariableTemplates` can be used with the help of the `*_vars.jl` files introduced here for ease in post-processing. In future PRs: - More abstraction and integration with Oceananigans diagnostics - Reducing MPI communication @slifer50 and @smarras79: please check the output for correctness relative to the previous PR. Cc: @tapios, @sandreza, @christophernhill, @blallen, @glwagner Co-authored-by: Kiran Pamnany <kpamnany@gmail.com>

bors · 2019-12-16T19:13:20Z

Build failed

ci/slurmci

kpamnany · 2019-12-16T19:21:37Z

bors r+

@slifer50

508: Diagnostics module r=kpamnany a=kpamnany Refactored implementation of the diagnostics prototyped in #467. This introduces the first version of an infrastructure for gathering diagnostics on the DG grid and moves the diagnostics implemented in #467 onto that infrastructure. It also moves the code into a CLIMA module. Diagnostics gathering is still triggered by the callback mechanism (and must be registered in the driver) until the proper timestepping mechanism is implemented. Output is to JLD2 and `VariableTemplates` can be used with the help of the `*_vars.jl` files introduced here for ease in post-processing. In future PRs: - More abstraction and integration with Oceananigans diagnostics - Reducing MPI communication @slifer50 and @smarras79: please check the output for correctness relative to the previous PR. Cc: @tapios, @sandreza, @christophernhill, @blallen, @glwagner Co-authored-by: Kiran Pamnany <kpamnany@gmail.com>

bors · 2019-12-16T20:36:39Z

Build failed

ci/slurmci

… the state array as well as some notable second moments average along the horizontal plane for each vertical level Refactored into CLIMA.Diagnostics. Add template for post-processing and graphing diagnostics output. Add test case. Support multiple ranks.

kpamnany · 2019-12-17T22:46:19Z

bors r+

@slifer50

508: Diagnostics module r=kpamnany a=kpamnany Refactored implementation of the diagnostics prototyped in #467. This introduces the first version of an infrastructure for gathering diagnostics on the DG grid and moves the diagnostics implemented in #467 onto that infrastructure. It also moves the code into a CLIMA module. Diagnostics gathering is still triggered by the callback mechanism (and must be registered in the driver) until the proper timestepping mechanism is implemented. Output is to JLD2 and `VariableTemplates` can be used with the help of the `*_vars.jl` files introduced here for ease in post-processing. In future PRs: - More abstraction and integration with Oceananigans diagnostics - Reducing MPI communication @slifer50 and @smarras79: please check the output for correctness relative to the previous PR. Cc: @tapios, @sandreza, @christophernhill, @blallen, @glwagner Co-authored-by: Kiran Pamnany <kpamnany@gmail.com>

bors · 2019-12-17T23:54:14Z

Build succeeded

ci/slurmci
CLIMA

kpamnany requested review from simonbyrne and charleskawczynski October 30, 2019 15:38

kpamnany requested a review from akshaysridhar October 30, 2019 15:43

blallen mentioned this pull request Nov 1, 2019

First pass at updating naming conventions. #446

Closed

tapios reviewed Nov 5, 2019

View reviewed changes

kpamnany mentioned this pull request Nov 5, 2019

Diagnostics #467

Closed

charleskawczynski reviewed Nov 5, 2019

View reviewed changes

src/Diagnostics/Diagnostics.jl Show resolved Hide resolved

charleskawczynski reviewed Nov 5, 2019

View reviewed changes

src/Diagnostics/Diagnostics.jl Outdated Show resolved Hide resolved

akshaysridhar reviewed Nov 5, 2019

View reviewed changes

src/Diagnostics/Diagnostics.jl Outdated Show resolved Hide resolved

kpamnany mentioned this pull request Nov 5, 2019

Sample diagnostics we will need #214

Open

charleskawczynski mentioned this pull request Nov 13, 2019

Add new MT constructor #532

Closed

kpamnany force-pushed the kp/diagnostics branch 2 times, most recently from b6fb65d to f31d71a Compare November 15, 2019 18:33

simonbyrne force-pushed the kp/diagnostics branch from b6fb65d to 2c7754d Compare November 17, 2019 03:49

simonbyrne force-pushed the master branch from 61364b7 to 5f2f29e Compare November 17, 2019 03:51

kpamnany force-pushed the kp/diagnostics branch 2 times, most recently from 25a7df6 to 9ce88cf Compare November 18, 2019 15:11

smarras79 reviewed Nov 18, 2019

View reviewed changes

kpamnany force-pushed the kp/diagnostics branch 2 times, most recently from baebf28 to 1e2d9cd Compare November 18, 2019 22:45

smarras79 reviewed Nov 19, 2019

View reviewed changes

src/Diagnostics/graph_diagnostic.jl Outdated Show resolved Hide resolved

smarras79 reviewed Nov 19, 2019

View reviewed changes

src/Diagnostics/graph_diagnostic.jl Outdated Show resolved Hide resolved

smarras79 reviewed Nov 19, 2019

View reviewed changes

src/Diagnostics/Diagnostics.jl Outdated Show resolved Hide resolved

kpamnany force-pushed the kp/diagnostics branch from 1e2d9cd to 9af3659 Compare November 22, 2019 16:04

kpamnany force-pushed the kp/diagnostics branch from 9af3659 to fc7b168 Compare November 22, 2019 18:49

blallen added the Diagnostics label Dec 2, 2019

kpamnany force-pushed the kp/diagnostics branch 6 times, most recently from 01b3fd7 to 26053a1 Compare December 13, 2019 21:28

kpamnany force-pushed the kp/diagnostics branch from 26053a1 to 3c07aba Compare December 16, 2019 19:20

kpamnany force-pushed the kp/diagnostics branch from 3c07aba to d0c81ec Compare December 17, 2019 21:37

bors bot merged commit d0c81ec into master Dec 17, 2019

bors bot deleted the kp/diagnostics branch December 17, 2019 23:54

yairchn mentioned this pull request Jun 10, 2022

Standard data format CliMA/ClimaCore.jl#747

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Diagnostics module #508

Diagnostics module #508

kpamnany commented Oct 30, 2019

kpamnany commented Oct 30, 2019

codecov bot commented Oct 30, 2019 •

edited

Loading

smarras79 commented Oct 30, 2019

tapios Nov 5, 2019

charleskawczynski Nov 5, 2019

smarras79 Nov 5, 2019 •

edited

Loading

tapios Nov 5, 2019

charleskawczynski Nov 15, 2019

smarras79 Nov 15, 2019

smarras79 commented Nov 5, 2019 via email

charleskawczynski Nov 5, 2019

kpamnany Nov 5, 2019

charleskawczynski Nov 5, 2019

smarras79 Nov 18, 2019

kpamnany Nov 18, 2019

akshaysridhar Nov 20, 2019

smarras79 Nov 19, 2019 •

edited

Loading

kpamnany Nov 22, 2019

charleskawczynski Nov 22, 2019

charleskawczynski Nov 22, 2019 •

edited

Loading

tapios Nov 22, 2019 •

edited

Loading

tapios Nov 22, 2019

kpamnany Nov 22, 2019

tapios Nov 22, 2019

kpamnany Nov 22, 2019

kpamnany commented Nov 26, 2019

kpamnany commented Dec 16, 2019

simonbyrne commented Dec 16, 2019

bors bot commented Dec 16, 2019

kpamnany commented Dec 16, 2019

bors bot commented Dec 16, 2019

kpamnany commented Dec 17, 2019

bors bot commented Dec 17, 2019


		e_int = e̅_tot - 1//2 * (u̅^2 + v̅^2 + w̅^2) - grav * zvals[k,ev]

		ts = PhaseEquil(convert(FT, e_int), q̅_tot, state.ρ)

Diagnostics module #508

Diagnostics module #508

Conversation

kpamnany commented Oct 30, 2019

kpamnany commented Oct 30, 2019

codecov bot commented Oct 30, 2019 • edited Loading

Codecov Report

smarras79 commented Oct 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarras79 Nov 5, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarras79 commented Nov 5, 2019 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarras79 Nov 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charleskawczynski Nov 22, 2019 • edited Loading

Choose a reason for hiding this comment

tapios Nov 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpamnany commented Nov 26, 2019

kpamnany commented Dec 16, 2019

simonbyrne commented Dec 16, 2019

bors bot commented Dec 16, 2019

Build failed

kpamnany commented Dec 16, 2019

bors bot commented Dec 16, 2019

Build failed

kpamnany commented Dec 17, 2019

bors bot commented Dec 17, 2019

Build succeeded

codecov bot commented Oct 30, 2019 •

edited

Loading

smarras79 Nov 5, 2019 •

edited

Loading

smarras79 Nov 19, 2019 •

edited

Loading

charleskawczynski Nov 22, 2019 •

edited

Loading

tapios Nov 22, 2019 •

edited

Loading