Skip to content

Rename to unambiguous pseudo- and geometric thickness and height names#327

Draft
xylar wants to merge 33 commits into
E3SM-Project:developfrom
xylar:omega/layer-thickness-to-pseudo-thickness
Draft

Rename to unambiguous pseudo- and geometric thickness and height names#327
xylar wants to merge 33 commits into
E3SM-Project:developfrom
xylar:omega/layer-thickness-to-pseudo-thickness

Conversation

@xylar
Copy link
Copy Markdown

@xylar xylar commented Dec 19, 2025

Checklist

  • Documentation:
  • Linting
  • Building
    • CMake build does not produce any new warnings from changes in this PR
  • Testing
    • Add a comment to the PR titled Testing with the following:
      • Which machines CTest unit tests
        have been run on and indicate that are all passing.
      • The Polaris omega_pr test suite
        has passed, using the Polaris e3sm_submodules/Omega baseline
      • Document machine(s), compiler(s), and the build path(s) used for -p for both the baseline (Polaris e3sm_submodules/Omega) and the PR build
      • Indicate "All tests passed" or document failing tests
      • Document testing used to verify the changes including any tests that are added/modified/impacted.
      • Performance related PRs: Please include a relevant PACE experiment link documenting performance before and after.

@xylar xylar self-assigned this Dec 19, 2025
@xylar xylar added the clean up label Dec 19, 2025
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This whole document needs some updating but I don't want to distract from the purpose of this PR to do it.

| ZMid | Real | NCellsSize, NVertLayers |
| GeopotentialMid | Real | NCellsSize, NVertLayers |
| LayerThicknessPStar | Real | NCellsSize, NVertLayers|
| PseudoThicknessPStar | Real | NCellsSize, NVertLayers|
Copy link
Copy Markdown
Author

@xylar xylar Dec 19, 2025

Choose a reason for hiding this comment

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

Here and in the user's guide, there's a reference to LayerThicknessPStar, now PseudoThicknessPStar, but I didn't see this in the code itself.

| ZMid | z height of layer midpoint | m |
| GeopotentialMid | geopotential at layer mid points | m$^2$/s$^2$|
| LayerThicknessPStar | desired layer thickness based on total perturbation from the reference thickness | - |
| PseudoThicknessPStar | desired layer thickness based on total perturbation from the reference thickness | - |
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Here and in the developer's guide, there's a reference to LayerThicknessPStar, now PseudoThicknessPStar, but I didn't see this in the code itself.

NDims, // number of dimensions
DimNames // dimension names
ZInterfFldName, // field name
"Geometric height at layer interfaces", // long name or description
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I feel pretty strongly that calling this the "Cartesian Z coordinate" is not correct -- the Cartesian coordinates for the MPAS mesh are in contrast to the spherical coordinates we usually use.

It also seems important to make clear that this is the geometric height (as opposed to the pseudo-height).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The other changes here are just to make the formatting consistent with the rest of the file.

NDims, // number of dimensions
DimNames // dimension names
ZMidFldName, // field name
"Geometric height at layer midpoints", // long name or description
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Same as with ZInterface above.

@xylar
Copy link
Copy Markdown
Author

xylar commented Dec 19, 2025

I know this will make for a rebasing nightmare for work in progress so I'll wait on this until we can coordinate a good time for it. I'll suck up the rebasing nightmare here, because it's a pretty simple search-and-replace job.

Comment thread components/omega/doc/userGuide/OceanState.md Outdated
Comment thread components/omega/doc/devGuide/AuxiliaryVariables.md Outdated
@xylar
Copy link
Copy Markdown
Author

xylar commented Dec 19, 2025

This needs to be tested in tandem with E3SM-Project/polaris#440, which is very much a work in progress. But CTests are passing as long as I use new meshes that include the PseudoThickness variable.

@xylar xylar force-pushed the omega/layer-thickness-to-pseudo-thickness branch from 931b9d5 to dc8fea0 Compare April 24, 2026 13:00
@xylar xylar changed the title Rename LayerThickness --> PseudoThickness Rename to unambiguous pseudo- and geometric thickness and height names Apr 24, 2026
@xylar xylar force-pushed the omega/layer-thickness-to-pseudo-thickness branch from dc8fea0 to 40fcd42 Compare May 4, 2026 16:40
@xylar xylar force-pushed the omega/layer-thickness-to-pseudo-thickness branch from 40fcd42 to 54631fb Compare May 6, 2026 11:36
@xylar xylar force-pushed the omega/layer-thickness-to-pseudo-thickness branch 2 times, most recently from a9dfcc2 to c713d0c Compare May 6, 2026 11:55
@xylar xylar mentioned this pull request May 6, 2026
14 tasks
brian-oneill and others added 10 commits May 7, 2026 20:56
@xylar xylar force-pushed the omega/layer-thickness-to-pseudo-thickness branch from d622b9f to e6c9041 Compare May 11, 2026 12:53
@xylar xylar force-pushed the omega/layer-thickness-to-pseudo-thickness branch from e6c9041 to 320f376 Compare May 11, 2026 12:59
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.

2 participants