Skip to content

Conversation

whannah1
Copy link
Contributor

@whannah1 whannah1 commented Apr 29, 2025

This phase focused on how ZM handles namelist parameters and constant values - building off of PR #7232 that created the zm_conv_types module to hold definitions and methods of the zm_const_t and zm_param_t derived types. Here we expand the use of these throughout the code, as well as move the zm_conv_readnl (originally zmconv_readnl) to the interface level to decrease the external dependencies needed to build zm_conv.F90, which should help make C++ bridging easier.

This PR also fixes the redundant namelist parameters for the DCAPE and ULL closure options, which originally had 3 parameters controlling 2 independent options. Now there will be a single flag for either option.

[BFB]

@whannah1 whannah1 added Atmosphere BFB PR leaves answers BFB ZM labels Apr 29, 2025
@whannah1 whannah1 marked this pull request as draft April 29, 2025 17:39
@whannah1 whannah1 force-pushed the whannah/eam/zm-cleanup-10 branch from 69af0d2 to f4b357d Compare May 1, 2025 18:58
@whannah1 whannah1 force-pushed the whannah/eam/zm-cleanup-10 branch from f4b357d to 2802668 Compare May 19, 2025 15:28
Copy link

github-actions bot commented May 19, 2025

PR Preview Action v1.6.1

🚀 View preview at
https://E3SM-Project.github.io/E3SM/pr-preview/pr-7300/

Built to branch gh-pages at 2025-06-25 17:40 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@whannah1 whannah1 marked this pull request as ready for review May 19, 2025 15:32
@whannah1 whannah1 marked this pull request as draft May 19, 2025 16:02
@whannah1
Copy link
Contributor Author

Note to self - this needs to stay in draft mode until PR #7281 is merged and this branch can be rebased.

@whannah1 whannah1 force-pushed the whannah/eam/zm-cleanup-10 branch from 2309211 to c49c2d3 Compare June 23, 2025 14:38
@whannah1 whannah1 marked this pull request as ready for review June 25, 2025 19:39
@whannah1
Copy link
Contributor Author

The following tests pass on Chrysalis, but with the expected "NLFAIL"

  e3sm_atm_developer_intel
  e3sm_atm_developer_gnu
  SMS_Ld32.ne30pg2_r05_oECv3.F2010.chrysalis_intel
  SMS_Ld32.ne30pg2_r05_oECv3.F2010.chrysalis_gnu
  SMS_Ld32.ne4pg2_oQU480.F2010.chrysalis_intel
  SMS_Ld32.ne4pg2_oQU480.F2010.chrysalis_gnu

@whannah1
Copy link
Contributor Author

The performance of the SMS_Ld32 tests are indentical

--------------------------------------------------------------------------------------------------------------------------------------------
  base: /lcrc/group/e3sm/ac.whannah/ZM_testing/20250625_193850_base/SMS_Ld32.ne30pg2_r05_oECv3.F2010.chrysalis_intel.G.20250625_143852_6ghfog
  test: /lcrc/group/e3sm/ac.whannah/ZM_testing/20250625_201834_test/SMS_Ld32.ne30pg2_r05_oECv3.F2010.chrysalis_intel.C.20250625_151836_jl3mxv

    base: ATM Run Time:    3725.434 seconds      116.420 seconds/mday         2.03 myears/wday
    test: ATM Run Time:    3714.811 seconds      116.088 seconds/mday         2.04 myears/wday

          name              on  processes  threads        count      walltotal   wallmax (proc   thrd  )   wallmin (proc   thrd  )
    base: "a_i:zm_convr"     -        128      256 1.536000e+03   5.560086e+00     0.028 (   126      0)     0.015 (   107      1)
    test: "a_i:zm_convr"     -        128      256 1.536000e+03   5.569709e+00     0.028 (    93      0)     0.015 (    52      0)

          name              on  processes  threads        count      walltotal   wallmax (proc   thrd  )   wallmin (proc   thrd  )
    base: "a:zm_convr"       -        128      256 2.360832e+06   1.267942e+04    56.590 (    59      1)    43.301 (   123      0)
    test: "a:zm_convr"       -        128      256 2.360832e+06   1.262210e+04    55.407 (     6      0)    42.905 (   100      1)

--------------------------------------------------------------------------------------------------------------------------------------------
  base: /lcrc/group/e3sm/ac.whannah/ZM_testing/20250625_193850_base/SMS_Ld32.ne30pg2_r05_oECv3.F2010.chrysalis_gnu.G.20250625_143852_gtwt5i
  test: /lcrc/group/e3sm/ac.whannah/ZM_testing/20250625_201834_test/SMS_Ld32.ne30pg2_r05_oECv3.F2010.chrysalis_gnu.C.20250625_151836_sli0db

    base: ATM Run Time:    4644.099 seconds      145.128 seconds/mday         1.63 myears/wday
    test: ATM Run Time:    4641.960 seconds      145.061 seconds/mday         1.63 myears/wday

          name              on  processes  threads        count      walltotal   wallmax (proc   thrd  )   wallmin (proc   thrd  )
    base: "a_i:zm_convr"     -        128      256 1.536000e+03   8.522980e+00     0.041 (    67      1)     0.024 (    52      0)
    test: "a_i:zm_convr"     -        128      256 1.536000e+03   8.492444e+00     0.041 (    93      0)     0.024 (    52      0)

          name              on  processes  threads        count      walltotal   wallmax (proc   thrd  )   wallmin (proc   thrd  )
    base: "a:zm_convr"       -        128      256 2.360832e+06   2.088901e+04    88.458 (     0      0)    73.041 (    91      1)
    test: "a:zm_convr"       -        128      256 2.360832e+06   2.065771e+04    87.131 (     0      0)    72.685 (    51      1)

--------------------------------------------------------------------------------------------------------------------------------------------

@rljacob
Copy link
Member

rljacob commented Jun 26, 2025

waiting on reviewer approval.

Copy link
Contributor

@tcclevenger tcclevenger left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@crterai crterai left a comment

Choose a reason for hiding this comment

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

With only nl changes and tests passing, looks good to me too.

singhbalwinder added a commit that referenced this pull request Jun 30, 2025
ZM cleanup - refactor handling of parameters and constants

This phase focused on how ZM handles namelist parameters and constant
values - building off of PR #7232 that created the zm_conv_types module
to hold definitions and methods of the zm_const_t and zm_param_t
derived types. Here we expand the use of these throughout the code, as
well as move the zm_conv_readnl (originally zmconv_readnl) to the
interface level to decrease the external dependencies needed to build
zm_conv.F90, which should help make C++ bridging easier.

This PR also fixes the redundant namelist parameters for the DCAPE and
ULL closure options, which originally had 3 parameters controlling 2
independent options. Now there will be a single flag for either option.

[BFB]

* whannah/eam/zm-cleanup-10:
  fix index for printing ZM derived types
  add missing zm_param to use statement
  cosmetic fix
  linter fix
  refactor zm_conv_readnl
@singhbalwinder
Copy link
Contributor

Merged to next

@singhbalwinder singhbalwinder merged commit 92a1738 into master Jul 1, 2025
9 checks passed
@singhbalwinder singhbalwinder deleted the whannah/eam/zm-cleanup-10 branch July 1, 2025 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Atmosphere BFB PR leaves answers BFB ZM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants