-
Notifications
You must be signed in to change notification settings - Fork 7
Add vertical coordinate design document #230
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
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a pretty straightforward class. I didn't go through the algorithm too carefully though it seems fine. Just had a few minor comments/suggestions.
|
||
### 2.1 Requirement: Compute and store the total pressure at a given level | ||
|
||
The total pressure at the bottom of each level will be computed and stored within the vertical coordinate module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the pressure at the mid-level for the pressure gradient and EOS. We do need the pressure at the bottom to represent the vertical coordinates in p. You have equations for both below, and it is a simple calculation. My main question is on memory versus flops. Should we have 3D variables for both pressure at the bottom and the mid-point? I think we could get away with pressure at the mid-layer, and just add (1/2 g h) when needed for the bottom. It is convenient to have both, of course. I would pose the question of memory versus convenience to the group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is something we should consider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another consideration. I would prefer to have the pressure at each layer interface (there is one more layer interface than the number of layers). This makes various computations (e.g. averaging or differencing) easier and it also doesn't arbitrarily favor the bottom vs. the top of a given layer. In Compass and Polaris, we have used z_interface
pretty extensively in this way.
I think we absolute have to have the pressure at interfaces (or bottoms of layers) because it is too difficult to reconstruct this from the pressure at the middle of layers. But we could consider always averaging to get the pressure at the middle of layers. If we do this, the best way to avoid having to treat the top and bottom of each layer as a special case would be to have pressure at all layer interfaces (including the top of the ocean).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another point, for the higher-order pressure gradient, it isn't obvious that we would want to compute the equation of state at the middle of layers as opposed to at quadrature points. So that may be another reason to prefer to just store it at the points where it is defined (layer interfaces).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to me to primarily store pressure and z at interfaces. We could compute the mid-layer values as needed for the centered pressure gradient and diagnostic output.
@katsmith133, are ZMid
and PressureMid
needed for vertical mixing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly gave minor edits/comments/suggestions as many of the major things have already been addressed. Also a few things addressing how the document renders once complied and shown on the web page.
aff8bd0
to
44bac61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sbrus89, this is really great! My comments are mostly about the finer points but I do have questions about the last 2 equations in section 3 that I'd like to chat about.
|
||
### 2.1 Requirement: Compute and store the total pressure at a given level | ||
|
||
The total pressure at the bottom of each level will be computed and stored within the vertical coordinate module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another consideration. I would prefer to have the pressure at each layer interface (there is one more layer interface than the number of layers). This makes various computations (e.g. averaging or differencing) easier and it also doesn't arbitrarily favor the bottom vs. the top of a given layer. In Compass and Polaris, we have used z_interface
pretty extensively in this way.
I think we absolute have to have the pressure at interfaces (or bottoms of layers) because it is too difficult to reconstruct this from the pressure at the middle of layers. But we could consider always averaging to get the pressure at the middle of layers. If we do this, the best way to avoid having to treat the top and bottom of each layer as a special case would be to have pressure at all layer interfaces (including the top of the ocean).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've contributed what I can to this document and I'm happy to see it get merged in (after the v1 equations, which it references) once others are also happy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
This PR adds a design document for the vertical coordinate module in Omega.
The compiled version is available here: https://portal.nersc.gov/project/e3sm/sbrus/vert_coord/html/design/VertCoord.html.
It will be kept up to date with changes from review comments.
Checklist