Skip to content

Fix nvhpc issue in #663 by moving chkErr use statement to the module level#668

Open
ekluzek wants to merge 2 commits into
ESCOMP:mainfrom
ekluzek:fix_nvhpc
Open

Fix nvhpc issue in #663 by moving chkErr use statement to the module level#668
ekluzek wants to merge 2 commits into
ESCOMP:mainfrom
ekluzek:fix_nvhpc

Conversation

@ekluzek

@ekluzek ekluzek commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Description of changes

Fix the problem with the nvhpc compiler where the use statement for chkErr needed to be at the module level to allow it's use throughout the model (alturnatively it could have been set in each procedure needing it).

Specific notes

Contributors other than yourself, if any:

CMEPS Issues Fixed (include github issue #):
Fixes #663
Fixes #666
Fixes #667

Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial) b4b

Any User Interface Changes (namelist or namelist defaults changes)? No

Testing performed

Ran aux_cmeps out of cesm3_0_alpha09d making sure for working tests it compares to the baseline I ran
cesm3_0_alpha09d
cmeps hash: 3b1fba9
cdeps with cdeps1.0.99

Tests ran as expected

ekluzek added 2 commits June 30, 2026 14:22
This addresses ESCOMP#667, and it uses a test that failed in ESCOMP#663.
@ekluzek ekluzek self-assigned this Jul 1, 2026
@ekluzek ekluzek added enhancement New feature or request CESM only Responsibility: CTSM Responsibility to manage and accomplish this issue is the CTSM Software group labels Jul 1, 2026

@billsacks billsacks 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.

I appreciate the fix for the nvhpc issue (it still seems likely to be a compiler issue to me, but I appreciate your submitting this workaround).

Can you give more justification for the test additions? I'm especially wondering about the addition of Fates tests. The aux_cmeps test list needs a major overhaul so I don't yet have clear ideas about what it should and shouldn't include, but my general philosophy would be to keep it fairly small to facilitate quick/cheap turnaround, and only add tests that give additional code coverage of the CMEPS code. Does the Fates test cover CMEPS code that isn't already covered by the existing I compset tests?

@billsacks

Copy link
Copy Markdown
Member

@ekluzek - based on our discussion earlier about how aux_cmeps needs a bigger, long-term overhaul, how would you feel about backing out at least the addition of the Fates tests. I'm on the fence about the other test changes you made: the removal of _P32 makes sense, at least.

@ekluzek

ekluzek commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

@ekluzek - based on our discussion earlier about how aux_cmeps needs a bigger, long-term overhaul, how would you feel about backing out at least the addition of the Fates tests. I'm on the fence about the other test changes you made: the removal of _P32 makes sense, at least.

I'm am amiable to removing the Fates test if you'd like me to. If the aux_cmeps testlist isn't really viable, I'm not sure it matters much. But, I'll take it out if you'd prefer.

@ekluzek

ekluzek commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

I appreciate the fix for the nvhpc issue (it still seems likely to be a compiler issue to me, but I appreciate your submitting this workaround).

I had assumed the problem was that chkErr was used in other subroutines besides the one. But, you are right they are all confined to one subroutine, so this change is not necessary. Hence, it must be a compiler issue for the nvhpc compiler. I figure it's not a bad change to make anyway, because having a basic utility like chkErr usable throughout the module is not a bad thing.

@ekluzek

ekluzek commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

I've done enough testing that I think this is ready to merge in. So @billsacks let me know if I should delete the Fates test, and then we can merge it in as the next tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CESM only enhancement New feature or request Responsibility: CTSM Responsibility to manage and accomplish this issue is the CTSM Software group

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Add some FATES-SP compset tests to aux_cmeps 32 task tests don't make sense... A few failing nvhpc compiler tests in cmeps1.1.48

2 participants