Skip to content

Use 2 timelevels for SCM state variables and remove the leapfrog scheme code - #717

Merged
grantfirl merged 6 commits into
NCAR:mainfrom
grantfirl:fix_state_memory_bug
Sep 16, 2026
Merged

grantfirl merged 6 commits into
NCAR:mainfrom
grantfirl:fix_state_memory_bug

Conversation

@grantfirl

@grantfirl grantfirl commented Aug 27, 2026 •

Copy link
Copy Markdown
Collaborator

SOURCE: @grantfirl CIRA

DESCRIPTION OF CHANGES:

  • Use state_X(timelevel=2) to hold the physics-updated state (gt0, gq0, etc) and state_X(timelevel=1) to hold physics-initial state (tgrs, qgrs). Set state_X(1) = state_X(2) at the end of the timestep. Note that this is now more similar to how UFS/FV3 handles the states. It wasn't previously necessary to have 2 different states, but the tendency update physics changes combined with physics schemes that handle prognostic tracers internally necessitate this change.
  • Remove the leapfrog scheme since it is no longer needed (hasnt been used since the transition to the DEPHY input format). This is just a cleanup and to avoid future maintenance and confusion.
  • Add only to module use statements in SCM code for better readability etc.

ISSUE: Fixes #184

ASSOCIATED PRs:
N/A

TESTS CONDUCTED: SCM RTs

REGRESSION TEST CHANGES: Using 2 timelevels to hold these different states causes machine-precision-level differences.

@grantfirl grantfirl changed the title activate timelevel 2 to hold updated physics state variables; transfe… Use 2 timelevels for SCM state variables and remove the leapfrog scheme code Aug 28, 2026
@grantfirl
grantfirl marked this pull request as ready for review August 28, 2026 18:31
@lisa-bengtsson

Copy link
Copy Markdown
Contributor

I just wanted to let you know that I tried this for the prognostic tracers I had included (updraft, area fraction), and it works. Thank you.

@grantfirl

Copy link
Copy Markdown
Collaborator Author

I just wanted to let you know that I tried this for the prognostic tracers I had included (updraft, area fraction), and it works. Thank you.

@lisa-bengtsson Great! Thanks for following up.

@scrasmussen scrasmussen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes look good! Like the use foo, only: bar changes. Looked through a number of the CI artifact plots that the CI said differed, but couldn't see any change from the baseline.

@hertneky hertneky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changes look good. I made comments about removal of some leftover mentions of leapfrog in pr718 on accident - mixing up the PRS! I decided to not make the same comments here.
Tested on derecho gnu: built and ran scm rts fine.

@grantfirl

Copy link
Copy Markdown
Collaborator Author

Changes look good. I made comments about removal of some leftover mentions of leapfrog in pr718 on accident - mixing up the PRS! I decided to not make the same comments here. Tested on derecho gnu: built and ran scm rts fine.

Thanks for the review. I addressed the comments in #718 related to the removal of the leapfrog scheme in this PR. The other comment will be addressed in that PR once we merge this one and update 718.

@grantfirl
grantfirl merged commit ea494c1 into NCAR:main Sep 16, 2026
17 checks passed
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.

Separate memory for statein and stateout variables

4 participants