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

Update NUOPC cap and externals to match CAM #262

Merged
merged 9 commits into from
Jun 6, 2024

Conversation

nusbaume
Copy link
Collaborator

This PR updates the NUOPC cap to match the version used in cam6_3_160, as well as the externals. Additional clean-up was also performed, including the removal of the now deprecated MCT cap.

Resolves #146

Tests run:

Ran Kessler and Held-Suarez snapshot comparison runs on Derecho with GNU.

Ran official regression tests (test_driver.sh) with both GNU and Intel on Derecho.

@nusbaume nusbaume added enhancement New feature or request code clean-up Made code simpler, better, and/or easier to read. labels May 20, 2024
@nusbaume nusbaume self-assigned this May 20, 2024
@nusbaume
Copy link
Collaborator Author

@cacraigucar @peverwhee when reviewing the src/cpl/nuopc/* files I would avoid trying to review them on Github, and instead just diff them against the same files that are currently in the CAM development branch to see what the differences currently are between SIMA and CAM. Of course if you have any questions or concerns with those files just let me know. Thanks!

Copy link
Collaborator

@cacraigucar cacraigucar left a comment

Choose a reason for hiding this comment

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

To find my comments, I did a mixture of xxdiff cam6_3_037 vs cam6_3_161 and xxdiff cam6_3_161 vs your cam-sima fork.

Externals.cfg Outdated Show resolved Hide resolved
src/cpl/nuopc/atm_stream_ndep.F90 Show resolved Hide resolved
@@ -83,7 +83,6 @@ module dyn_grid
public :: model_grid_init
Copy link
Collaborator

Choose a reason for hiding this comment

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

In this routine, I see:

  • use_cslam was instituted as a replacement for fv_nphys>0
  • line 256 no longer had =thermodynamic_active_species_num+3, but simply has =3
  • at line 400, the GLL grid no longer has the 'area' attribute, just 'np' and 'ne'
  • fvm_aerawt and physgrid_areawt were added in a number of places

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep all of those changes will be coming in with my future SE dycore update PR (as each one of them will require non-trivial changes to the dycore code itself).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Leaving this unresolved, just so I can find it again, but agree to deferring

src/control/cam_comp.F90 Show resolved Hide resolved
src/cpl/nuopc/atm_comp_nuopc.F90 Show resolved Hide resolved
src/cpl/nuopc/atm_comp_nuopc.F90 Show resolved Hide resolved
src/cpl/nuopc/atm_comp_nuopc.F90 Show resolved Hide resolved
src/control/cam_control_mod.F90 Show resolved Hide resolved
src/cpl/nuopc/atm_import_export.F90 Show resolved Hide resolved
@nusbaume nusbaume requested a review from cacraigucar May 23, 2024 22:46
@@ -83,7 +83,6 @@ module dyn_grid
public :: model_grid_init
Copy link
Collaborator

Choose a reason for hiding this comment

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

Leaving this unresolved, just so I can find it again, but agree to deferring

Copy link
Collaborator

@peverwhee peverwhee left a comment

Choose a reason for hiding this comment

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

A few small things!

I also think someone recently added a "SHOULD" to the CAM Fortran coding standard to keep use statements at the lowest scope - something to consider.

Thanks @nusbaume !

src/cpl/nuopc/atm_comp_nuopc.F90 Outdated Show resolved Hide resolved
src/cpl/nuopc/atm_import_export.F90 Show resolved Hide resolved
src/cpl/nuopc/atm_stream_ndep.F90 Outdated Show resolved Hide resolved
src/cpl/nuopc/atm_stream_ndep.F90 Outdated Show resolved Hide resolved
@nusbaume
Copy link
Collaborator Author

nusbaume commented Jun 6, 2024

@peverwhee Just FYI I also did some use statement re-arrangement to try and better match the Fortran coding standards for CAM-SIMA. I didn't completely finish the effort though, mostly to avoid making comparisons with the original CAM code overly difficult.

@nusbaume nusbaume requested a review from peverwhee June 6, 2024 16:50
Copy link
Collaborator

@peverwhee peverwhee left a comment

Choose a reason for hiding this comment

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

thanks @nusbaume !

@nusbaume nusbaume merged commit b7d9dbb into ESCOMP:development Jun 6, 2024
6 checks passed
@nusbaume nusbaume deleted the nuopc_update branch June 12, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code clean-up Made code simpler, better, and/or easier to read. enhancement New feature or request
Projects
Status: Tag
Development

Successfully merging this pull request may close these issues.

3 participants