Skip to content
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

Feature/diagnostics level select #370

Merged
merged 10 commits into from
Nov 1, 2022
Merged

Conversation

elynnwu
Copy link
Collaborator

@elynnwu elynnwu commented Oct 28, 2022

Purpose

This PR adds the capability of saving a vertical slice using z_select in the diagnostics config.

The expected syntax is as follows:

diagnostics_config:
  path: output
  output_format: netcdf
  z_select:
    - level: 1
       names:
         - ua
         - va
    - level: 10
       names:
         - pt  

Code changes:

  • diagnostics.py: added ZSelect class for storing selected vertical slice and the variables to be saved
    • fix column integral mistake, need to multiple by 1/gravity

Checklist

Before submitting this PR, please make sure:

  • You have followed the coding standards guidelines established at Code Review Checklist.
  • Docstrings and type hints are added to new and updated routines, as appropriate
  • All relevant documentation has been updated or added (e.g. README, CONTRIBUTING docs)
  • For each public change and fix in pace-util, HISTORY has been updated
  • Unit tests are added or updated for non-stencil code changes

@elynnwu elynnwu marked this pull request as ready for review October 31, 2022 21:22
Copy link
Contributor

@twicki twicki left a comment

Choose a reason for hiding this comment

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

just a few questions

@@ -33,6 +33,10 @@ diagnostics_config:
- qrain
- qsnow
- qgraupel
z_select:
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we decide to re-think the layout in the config in a later PR? Or did you and @mcgibbon come to this as the final look?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We converged to this layout because it is saved as a separate variable instead of as part of the snapshot.

level: int
names: List[str]

def slice_data(self, state: DycoreState):
Copy link
Contributor

Choose a reason for hiding this comment

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

technically we select and don't slice here - not super relevant as selecting is a 1-sized slice, but if we actually wan to implement both we might want to rename it here already?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed it to select data.

@elynnwu elynnwu enabled auto-merge (squash) November 1, 2022 18:14
@elynnwu elynnwu merged commit aeb84e1 into main Nov 1, 2022
@elynnwu elynnwu deleted the feature/diagnostics-level-select branch November 1, 2022 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants