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

Fix units and description for rt_diabatic_tend in atmosphere core Registry.xml #1092

Conversation

mgduda
Copy link
Contributor

@mgduda mgduda commented Jun 28, 2023

This PR corrects the units and description for the rt_diabatic_tend field in the atmosphere Registry.xml.

The rt_diabatic_tend field represents the tendency of the modified potential temperature due to cloud microphysics in K/s; accordingly, this commit corrects the units and description attributes for rt_diabatic_tend in the atmosphere core's Registry.xml file.

…istry.xml

The rt_diabatic_tend field represents the tendency of the modified potential
temperature due to cloud microphysics in K/s; accordingly, this commit corrects
the units and description attributes for rt_diabatic_tend in the atmosphere
core's Registry.xml file.
@mgduda mgduda marked this pull request as ready for review June 28, 2023 16:01
@mgduda mgduda requested review from ldfowler58 and smileMchen June 28, 2023 16:01
Copy link

@smileMchen smileMchen left a comment

Choose a reason for hiding this comment

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

This change is correct.

@ldfowler58
Copy link
Contributor

I am wondering if it would make sense to move rt_diabatic_tend from "tend" to "tend_physics" in Registry.xml. My reasoning is twofold:

  1. rt_diabatic_tend is actually a physics-related tendency from cloud microphysics processes, and it is an uncoupled variable as the other physics tendencies in "tend_physics".

  2. All the variables in "tend" are coupled tendencies but rt_diabatic_tend is not.

Considering that rt_diabatic_tend is only needed in subroutine atm_recover_large_step_variables, it may not be too much work to do this.

Michael, if you are OK to consider it, I am happy to add a PR to include this change. Thanks.

@mgduda
Copy link
Contributor Author

mgduda commented Jun 29, 2023

@ldfowler58 I don't have a strong opinion on where rt_diabatic_tend is stored, but I think you make a good argument for moving it to the tend_physics pool/var_struct. As you said, that would be a separate PR, which would target the develop branch.

For this PR specifically, do the changes to the units and description attributes look good to you?

@mgduda mgduda merged commit 1974dc0 into MPAS-Dev:hotfix-v8.0.1 Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants