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

Variance dissipation computation #3877

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

simone-silvestri
Copy link
Collaborator

This PR introduces a metric to compute the implicit dissipation introduced by flux form advection schemes.
This metric has been devised by @jm-c and is still subject of some tests + writeup of the method, so this PR is still very much a work in progress.

@glwagner
Copy link
Member

How about TracerVarianceDissipation, omitting Computation?

@tomchor
Copy link
Collaborator

tomchor commented Nov 4, 2024

@simone-silvestri this is great! Looking forward to trying this out!

One quick question: since this is a diagnostic quantity, would it make sense to include this on Oceanostics instead of Oceananigans?

Comment on lines 54 to 55
# Note: This works only if the callback is called with an IterationInterval(1), if not the
# previous fluxes and velocities will not be correct
Copy link
Member

Choose a reason for hiding this comment

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

You can actually enforce this by extending the Callback constructor

Comment on lines +192 to +194
include("VarianceDissipationComputation/VarianceDissipationComputation.jl")

using .VarianceDissipationComputation
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
include("VarianceDissipationComputation/VarianceDissipationComputation.jl")
using .VarianceDissipationComputation
include("VarianceDissipations/VarianceDissipations.jl")
using .VarianceDissipations

Copy link
Member

@glwagner glwagner Nov 7, 2024

Choose a reason for hiding this comment

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

This is explicitly the julia convention for modules whose primary purpose is to introduce a type


prod_names = Tuple(Symbol(:A, tracer_name, dir) for dir in dirs)
diff_names = Tuple(Symbol(:D, tracer_name, dir) for dir in dirs)
grad_names = Tuple(Symbol(:G, tracer_name, dir) for dir in dirs)
Copy link
Member

Choose a reason for hiding this comment

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

wut

diffusive_prod = Tuple(getproperty(D, dir) for dir in dirs)
grad = Tuple(getproperty(G, dir) for dir in dirs)

return NamedTuple{tuple(prod_names..., diff_names..., grad_names...)}(tuple(advective_prod..., diffusive_prod..., grad...))
Copy link
Member

Choose a reason for hiding this comment

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

ummm

you may have meant to write something like

names = tuple(prod_names..., diff_names..., grad_names...)

z = ZFaceField(grid)

return (; x, y, z)
end
Copy link
Member

Choose a reason for hiding this comment

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

does this help?


Vⁿ.x[i, j, k] = _diffusive_tracer_flux_x(i, j, k, grid, clo, K, Val(c_id), c, clk, fields, b) * Axᶠᶜᶜ(i, j, k, grid)
Vⁿ.y[i, j, k] = _diffusive_tracer_flux_y(i, j, k, grid, clo, K, Val(c_id), c, clk, fields, b) * Ayᶜᶠᶜ(i, j, k, grid)
Vⁿ.z[i, j, k] = _diffusive_tracer_flux_z(i, j, k, grid, clo, K, Val(c_id), c, clk, fields, b) * Azᶜᶜᶠ(i, j, k, grid)
Copy link
Member

Choose a reason for hiding this comment

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

missing @inbounds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants