Skip to content

Conversation

@PeterHjortLauritzen
Copy link
Collaborator

Tag name (required for release branches):
Originator(s):

Description (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number):

Describe any changes made to build system:

Describe any changes made to the namelist:

List any changes to the defaults for the input datasets (e.g. boundary datasets):

List all files eliminated and why:

List all files added and what they do:

List all existing files that have been modified, and describe the changes:
(Helpful git command: git diff --name-status development...<your_branch_name>)

If there are new failures (compared to the test/existing-test-failures.txt file),
have them OK'd by the gatekeeper, note them here, and add them to the file.
If there are baseline differences, include the test and the reason for the
diff. What is the nature of the change? Roundoff?

derecho/intel/aux_sima:

derecho/gnu/aux_sima:

If this changes climate describe any run(s) done to evaluate the new
climate in enough detail that it(they) could be reproduced:

CAM-SIMA date used for the baseline comparison tests if different than latest:

Copy link
Member

@jimmielin jimmielin left a comment

Choose a reason for hiding this comment

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

Thanks @PeterHjortLauritzen! Some early comments.

I think there are a handful of instances in the dycore that have code walled off using #ifdef 0 and a comment, for much of the code that depends on tracers/chemistry/constituents and history, which were previously unsupported. Not sure if we should begin to implement these now or continue to wall these off (perhaps in a constituent manner to what is there now, i.e., change all to #ifdef 0) so they can be dealt with together at a later time.

dry surface pressure scaling. If less than or equal to 0.0,
do not perform scaling. If greater than 0.0, perform scaling to scale_dry_air_mass
value (in Pa) as the average dry surface pressure target.
Default: set by build-namelist.
Copy link
Member

Choose a reason for hiding this comment

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

Build-namelist is no longer present after migrating to CAM-SIMA so this could be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there are other places in this file with ... so should I move this description elsewhere?

Copy link
Member

Choose a reason for hiding this comment

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

@nusbaume might have thoughts about this but I generally just left this line out when moving over the description from CAM

! Ensure N2 = 1 - (O2 + O + H) mmr is greater than 0
! Check for unusually large H2 values and set to lower value.
!------------------------------------------------------------
!xxx this code is NOT in cam_development?
Copy link
Member

Choose a reason for hiding this comment

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

I think this was migrated to physics_cnst_limit in cam_development. I remember taking a look at this when porting the check_energy infrastructure and the code seemed the same, just that it is wrapped in a subroutine in CAM (physics_cnst_limit in physics_types).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nusbaume @jimmielin Can I remove this comment?

Comment on lines +33 to +36
#ifdef constituents
use constituents, only: pcnst, cnst_name, cnst_longname
use dimensions_mod, only: fv_nphys, cnst_name_gll, cnst_longname_gll, qsize
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Now that the CAM-SIMA history is implemented I think this subroutine could be modified to use the constituent properties pointer, not sure if it will fully work but please let me know if you like me to give it a try!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it would be great to get some help ...

Copy link
Member

Choose a reason for hiding this comment

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

As mentioned during the SE coordination meeting we could bring these in at a later date - I've created an issue as a reminder #446. Thanks!

@jimmielin
Copy link
Member

Note -- maybe it could be addressed in this PR: To support CAM7 moving mountain gravity waves the vort4gw code needs to be ported from CAM to the SE dycore code here as well (instead of using the pbuf just populate vorticity in physics_state so it is visible to the CCPP schemes via the CAM-SIMA registry)

@PeterHjortLauritzen
Copy link
Collaborator Author

PeterHjortLauritzen commented Nov 24, 2025

FYI: In FKESSLER (10 day test) solution is B4B with cam_development tag cam6_4_089 when performance enhacement PR has been committed (ESCOMP/CAM#1365)

Copy link
Member

@jimmielin jimmielin left a comment

Choose a reason for hiding this comment

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

Thanks @PeterHjortLauritzen!

Since this is bit-for-bit with cam_development and I have no further questions about the code I'll approve it from my end! Depending on whether this or the gravity waves PR goes in first I'm happy to handle whatever merge conflicts arise due to vorticity code updates in the GW PR that also affect the SE dycore.

dry surface pressure scaling. If less than or equal to 0.0,
do not perform scaling. If greater than 0.0, perform scaling to scale_dry_air_mass
value (in Pa) as the average dry surface pressure target.
Default: set by build-namelist.
Copy link
Member

Choose a reason for hiding this comment

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

@nusbaume might have thoughts about this but I generally just left this line out when moving over the description from CAM

Comment on lines +33 to +36
#ifdef constituents
use constituents, only: pcnst, cnst_name, cnst_longname
use dimensions_mod, only: fv_nphys, cnst_name_gll, cnst_longname_gll, qsize
#endif
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned during the SE coordination meeting we could bring these in at a later date - I've created an issue as a reminder #446. Thanks!

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.

3 participants