Skip to content

Rename LayerThickness --> PseudoThickness#327

Draft
xylar wants to merge 6 commits intoE3SM-Project:developfrom
xylar:omega/layer-thickness-to-pseudo-thickness
Draft

Rename LayerThickness --> PseudoThickness#327
xylar wants to merge 6 commits intoE3SM-Project:developfrom
xylar:omega/layer-thickness-to-pseudo-thickness

Conversation

@xylar
Copy link

@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
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
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
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
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
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
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
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.

## Ocean State

The `OceanState` class provides a container for the non-tracer prognostic variables in Omega, namely `normalVelocity` and `layerThickness`.
The `OceanState` class provides a container for the non-tracer prognostic variables in Omega, namely `normalVelocity` and `PseudoThickness`.
Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
The `OceanState` class provides a container for the non-tracer prognostic variables in Omega, namely `normalVelocity` and `PseudoThickness`.
The `OceanState` class provides a container for the non-tracer prognostic variables in Omega, namely `NormalVelocity` and `PseudoThickness`.

Comment on lines -92 to 93
| LayerThicknessAuxVars | FluxLayerThickEdge | Center or Upwind|
| PseudoThicknessAuxVars | FluxLayerThickEdge | Center or Upwind|
|| MeanLayerThickEdge ||
Copy link
Author

Choose a reason for hiding this comment

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

Do we also need to rename FluxLayerThickEdge and MeanLayerThickEdge?

@xylar
Copy link
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.

The new ones include as many Omega variable names as possible
and have the `PseudoThickness` variable added.
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.

1 participant