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

Docstring for ShallowWaterScalarDiffusivity #2941

Merged
merged 4 commits into from
Feb 25, 2023
Merged

Conversation

navidcy
Copy link
Collaborator

@navidcy navidcy commented Feb 22, 2023

I made an attempt. But it can be definitely improved...

So this diffusivity is applied to the dynamics regardless the formulation?

@francispoulin can you edit the docstring? Or @simone-silvestri?

Closes #2939

@navidcy navidcy changed the title Docstring needed for ShallowWaterScalarDiffusivity Docstring for ShallowWaterScalarDiffusivity Feb 22, 2023
@navidcy navidcy added the documentation 📜 The sacred scrolls label Feb 22, 2023
Copy link
Collaborator

@francispoulin francispoulin left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you @navidcy for doing this.

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 24, 2023

So is what I wrote ok? Does it apply for both formulations or should we distinguish?

@simone-silvestri
Copy link
Collaborator

It applies to both formulations. sw_∂ⱼ_τ₁ⱼ uses fields u and v to calculate t. Both formulations have a u and v field, in the ConservativeFormulation, they are diagnostic quantities while in the VectorInvariant they are the prognostic velocity

function shallow_water_velocities(solution, formulation)
if formulation isa VectorInvariantFormulation
return (u = solution.u, v = solution.v, w = nothing)
else
u = Field(@at (Face, Center, Center) solution.uh / solution.h)
v = Field(@at (Center, Face, Center) solution.vh / solution.h)
compute!(u)
compute!(v)
return (; u, v, w = nothing)
end
end
shallow_water_fields(velocities, solution, tracers, ::ConservativeFormulation) = merge(velocities, solution, tracers)
shallow_water_fields(velocities, solution, tracers, ::VectorInvariantFormulation) = merge(solution, (; w = velocities.w), tracers)

@simone-silvestri
Copy link
Collaborator

The only distinction is that in the VectorInvariantFormulation (that evolves $u$ and $v$) you want to compute
$h^{-1} \nabla \nu h \nabla t$,
while in the ConservativeFormulation (that evolves $uh$ and $vh$)
$\nabla \nu h \nabla t$

@navidcy
Copy link
Collaborator Author

navidcy commented Feb 24, 2023

Ok. Will add a remark and merge ;)

@navidcy navidcy merged commit e315a73 into main Feb 25, 2023
@navidcy navidcy deleted the ncc/shallowwaterdiffusivity branch February 25, 2023 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation 📜 The sacred scrolls
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docstring needed for ShallowWaterScalarDiffusivity
3 participants