Add optional vertical Coriolis term to hydrostatic pressure integral #4594
Conversation
glwagner
changed the title
|
|
||
| for k in grid.Nz-1 : -1 : 1 | ||
| @inbounds pHY′[i, j, k] = pHY′[i, j, k+1] - z_dot_g_bᶜᶜᶠ(i, j, k+1, grid, buoyancy, C) * Δzᶜᶜᶠ(i, j, k+1, grid) | ||
| dpdz⁺ = z_dot_g_bᶜᶜᶠ(i, j, k+1, grid, buoyancy, tracers) - z_f_cross_U(i, j, k+1, grid, coriolis, velocities) |
There was a problem hiding this comment.
This looks great! Does z_f_cross_U need to be defined anywhere or is it already computed?
There was a problem hiding this comment.
This looks good to me and I approve the changes, but I think someone else should too.
To add in the complete Coriolis force we just need to add another body force in the zonal momentum equation. That should be easy enough to do without worrying about whether the methods are energy or enstrophy conserving.
why do we need another body force? Isn't this already included in x_cross_U? We already have non-traditional models for example. You could set up an example with a non-traditional Coroilis in the hydrostatic model to validate this |
|
for example Oceananigans.jl/src/Coriolis/constant_cartesian_coriolis.jl Lines 68 to 69 in 6c2944e this is the complete tangent plane approximation for any background rotation around an arbitrary axis. There is also a non-traditional beta plane. |
This is probably my misunderstanding. I just meant that in the zonal momentum equation we need a term like If it's there already, great. If not, it's easy to add it in. We need this term to cancel the term added into the vertical momentum equation so that energy is conserved. Maybe it's there already but I didn't see it. When I look in the spherical Coriolis script, I see that it only computes the sin of the latitude, not the cosine. But this should be easy to copy over from the non-traditional term, as you mentioned. |
There are two things going on and I would like to implement them in separate PRs. First, this PR implements a z-component of the Coriolis force which can be used in the hydrostatic model (we already have this for Since we already have these implemented, a simulation that uses these types in There is finally an additional Coriolis type we can add, which includes the full spherical variation of the Coriolis force with latitude in addition to the extra vertical and zonal components of the Coriolis force that do not appear in the typical This second addition is beyond the scope of this PR and should be tackled in another PR. |
|
Just to clarify further, it is only models on |
|
Thanks @glwagner for clarifying. I'm happy with these two parts, and am glad we are starting with the easier of the two. I just want to repeat that if we include the Coriolis term in the vertical equation, so we have hydrostatic balance, we should always make sure to include the other non-traditional term in the zonal momentum equation. If you don't include both your energy balance will be all messed up. |
But that is guaranteed by every Coriolis implementation. So what is the problem? |
|
Here are two examples:
|
Not a problem, I don't think. |
In the first term I don't now where we have The second term looks to be correct but I don't know where the terms like |
The To include the second term, we need a new Coriolis type. In other words, there is no problem now, because there is no Coriolis implementation that includes a vertical component but then inconsistently does NOT include the corresponding zonal term. Note, there is no where else int he code where Coriolis forces are defined. The entire Coriolis force must be defined through the |
|
@glwagner I added a file in examples called This still needs to be set up to validate that this PR does what it wants, but is this what you were thinking? It's easy enough to have either the traditional or nontraditional terms, or both. I suppose it would be worth testing what this done on main before the choices? |
|
@glwagner , did you have a chance to look at the file I added into examples? This should not be an example, but it's something we can play with, I think. It specifies a mean velocity in two directions that depend on both components of the Coriolis parameters. I thought this is what you had asked about. |
|
What's the output of the file? Does it validate the new Coriolis term? |
No. At the moment it just sets up a thermal wind problem with the traditional and non-traditioinal Coriolis terms. It is using the Cartesian geometry, just as a starting point and will be useful to build from and then for comparison. I will extend to to a lat-lon grid and see how things work, if they work. I just wanted to check to see if this is what you had in mind first. I can work on this on Monday or Tuesday for the spherical geometry. |
|
Does the script run? |
This PR adds the vertical Coriolis term, z_f_cross_U, to the hydrostatic pressure integral.
I implemented this so that it is optional. This way, the hydrostatic model can use it, but the nonhydrostatic model will not. We don't want to use this for the nonhydrostatic model because we don't always compute the hydrostatic pressure.
I did a bit of housecleaning too.
cc @francispoulin
@simone-silvestri I would like to change the kwarg "parameters" to "kernel_parameters" everywhere in a future PR (just mentioning here since I was looking at it).
Also the kernel parameter functions should not be defined in the same file as the kernels. They should be defined where they are used, eg for each model.