Skip to content

Conversation

@brian-oneill
Copy link

This PR adds the design document for the vertical advection module. Compiled here.

Checklist

  • Documentation:
    • Design document has been generated and added to the docs
    • Documentation has been built locally and changes look as expected

@cbegeman
Copy link

cbegeman commented Dec 1, 2025

Can you clarify somewhere the relationship between your "Core data arrays" and the variables in the Algorithm section? I think I understand VerticalTransport to correspond to $\tilde{W}_{tr}$ but I don't understand why that is a 2-d array. Is VerticalFlux intended to hold either $U \tilde{W}_{tr}$ or $\phi \tilde{W}_{tr}$?

@cbegeman
Copy link

cbegeman commented Dec 1, 2025

The description of VertAdv::computeVerticalTransport sounds to me like it's computing $\tilde{w}$, but I think we've been using the word "Transport" to denote $\tilde{W}_{tr}$. Can you clarify where we're computing each and whether both will be available for output?

@brian-oneill
Copy link
Author

@cbegeman Looking again at the V1 equations document, there is a table where VerticalTransport is used for mass flux, and VerticalVelocity is the velocity, so I'll rename the array here to VerticalVelocity (for $\tilde{w}$). VerticalFlux is just intended for tracer fluxes (specifically the high-order fluxes in FCT, and in the standard vertical advection algorithm there is only one flux array needed).

My initial thoughts were to just modify VerticalVelocity in place to include the corrections (for moving coordinate, projection of normal velocity, etc). If we want to store $\tilde{w}$ and $\tilde{W}_{tr}$ separately to allow for output of both quantities I can add a TotalVerticalVelocity member array. As for the actual computation of these corrections, I haven't worked out that implementation yet. For the initial PR, I was planning to assume $\tilde{W}_{tr} = \tilde{w}$.

@cbegeman
Copy link

cbegeman commented Dec 2, 2025

@brian-oneill Thanks for clarifying. I think it would be helpful to be explicit in the document with math notation where you are solving for each. The assumption of no vertical coordinate motion seems fine, but it would probably be helpful to specify that in a few places in the document as well. I haven't given this a lot of thought to say whether modifications in place is the best way forward, but it does seem like adding the projection of the normal velocity to VerticalVelocity could lead to confusion.

Copy link
Collaborator

@sbrus89 sbrus89 left a comment

Choose a reason for hiding this comment

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

This looks good @brian-oneill, I just have a couple comments.

$i$ is computed from the thickness-weighted divergence of horizontal velocity:

$$
\tilde{w}_{i,k-1/2} = \tilde{w}_{i,k+1/2} +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we are planning on using the "p-star" coordinate, should we explicitly define $$\tilde{W}$$ to include the ALE term? That would mean adding the pseudo-height/velocity equivalent of equations (4) and (5) from Petersen et al. 2015.

configuration is used to compute interface fluxes, which are then applied to
update the tracer tendencies.

The `computeFCTVAdvTend` method implements the flux-corrected transport scheme
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe make it more clear here and in Section 3.4 that FCT is only for tracers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants