Skip to content

Baroclinic Steady Test Case Initialization#64

Merged
jjuyeonkim merged 6 commits intoNOAA-GFDL:developfrom
jjuyeonkim:20250618_baroclinic12_ss_2_clean
Jul 9, 2025
Merged

Baroclinic Steady Test Case Initialization#64
jjuyeonkim merged 6 commits intoNOAA-GFDL:developfrom
jjuyeonkim:20250618_baroclinic12_ss_2_clean

Conversation

@jjuyeonkim
Copy link
Collaborator

@jjuyeonkim jjuyeonkim commented Jul 7, 2025

Description

  • This PR will add DycoreState initialization for the Baroclinic steady state test case found in tools/test_cases.F90 (test case 12) of the GFDL_atmos_cubed_sphere repo. Because the Baroclinic instability test case (13) already exists, this test case will piggy-back heavily off of the existing code.

  • A new test case ""baroclinic_steady"" has been added. The previously existing ""baroclinic"" test case has been changed to ""baroclinic_instability"".

  • The Analytic test case has been changed from a string to an enumeration.

  • Because code require additional changes within Pace, the pace tests have been commented out for now. See related PR: Baroclinic Steady State Test Case pace#129

  • Mypy is being turned off for a initialize_rossby.py until further investigation can be made. The pre-commit checks from PyFV3 pass, but they fail in Pace.

Fixes # (issue)
None

How Has This Been Tested?
There are only basic unit tests being added in the pace repository, which uses PyFV3 as a submodule. This related code will be added in a separate PR for the Pace repo.

In addition to the unit tests, a comparison was made to zero time SHiELD results. The resulting max differences after initialization between SHiELD and pace results for phis, delp, u, v attributes were on the order of 10^-12.
C48.solo.BCmoist.pace_test12_64_debug

However, a comparison was made to 1-day SHiELD results: C48.solo.BCmoist.pace_test12_64_1day_dycoreonly_debug After 1 day, there was a difference in the pace results compared to SHiELD that needs further investigation. This difference also exists in the Baroclinic instability test case that existed prior to this PR.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (Not Applicable)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules (Code changes here affect the Pace repository. After Pace changes have been pushed, we will need to turn the pace tests back on.)
  • New check tests, if applicable, are included (Tests will be added with a related PR in the pace repository.)
  • Targeted model if this changed was triggered by a model need/shortcoming (Not applicable)

Copy link

@bensonr bensonr left a comment

Choose a reason for hiding this comment

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

lgtm

@jjuyeonkim jjuyeonkim marked this pull request as ready for review July 8, 2025 15:36
@fmalatino fmalatino self-requested a review July 8, 2025 17:39
fmalatino
fmalatino previously approved these changes Jul 8, 2025
Copy link
Contributor

@fmalatino fmalatino left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Collaborator

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

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

I'd like to have a go at that mypy issue, this could be a canary in the coalmine of a wider issue of API

Copy link
Collaborator

@oelbert oelbert left a comment

Choose a reason for hiding this comment

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

Nice job

Copy link
Collaborator

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

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

LGTM

@jjuyeonkim jjuyeonkim merged commit 95c62c0 into NOAA-GFDL:develop Jul 9, 2025
2 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.

5 participants