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

replace esmf_logmsg_error with calls to shr_abort #532

Merged
merged 8 commits into from
Jan 29, 2025

Conversation

jedwards4b
Copy link
Collaborator

@jedwards4b jedwards4b commented Jan 23, 2025

Description of changes

Replace calls to ESMF_LogWrite that preceed an error exit with calls to shr_sys_abort which will include the error message in both the ESMF pet file and
the component log for the appropriate component. This requires an update to the cesm_share code as well.

Specific notes

Fixes #527
Fixes #531
Fixes #455

Contributors other than yourself, if any:

CMEPS Issues Fixed (include github issue #):

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

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

Testing performed

Please describe the tests along with the target model and machine(s)
If possible, please also added hashes that were used in the testing

@jedwards4b jedwards4b self-assigned this Jan 23, 2025
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

@jedwards4b thanks so much for taking this on! This is great to see come in quickly.

I saw a couple things to handle and some questions that I bring up. I also saw some additional instances of this pattern to handle. I think doing...

git grep ESMF_LOGMSG_ERROR

and removing all instances of that is the best way to go. In some cases it's now just in "use" statements, but there is no reason to keep it around there either.

cime_config/testdefs/testlist_drv.xml Show resolved Hide resolved
cime_config/testdefs/testlist_drv.xml Show resolved Hide resolved
cime_config/testdefs/testlist_drv.xml Outdated Show resolved Hide resolved
cime_config/testdefs/testlist_drv.xml Show resolved Hide resolved
mediator/med_methods_mod.F90 Show resolved Hide resolved
@jedwards4b jedwards4b requested a review from ekluzek January 28, 2025 19:31
@billsacks
Copy link
Member

I'm personally happy with this change, though want to get feedback from others in the ESMF team tomorrow in case someone sees issues with this that I may not be seeing.

One thing I'm wondering is whether it's safe to introduce this dependence on shr_sys_mod - for the sake of systems other than CESM. I see that some other uses of shr modules are protected by ifdefs, though not all, so I'm not sure what the general rule is. The one pre-existing use of shr_sys_abort was actually one that I added in #471 , but I suspect I didn't think it through carefully at the time.

@jedwards4b
Copy link
Collaborator Author

@billsacks this (actually the same changes in cdeps) has already been reviewed and tested by @DeniseWorthen. So the changes are fine for UFS.

@billsacks
Copy link
Member

this (actually the same changes in cdeps) has already been reviewed and tested by @DeniseWorthen. So the changes are fine for UFS.

Ah, sounds great - thanks!

@jedwards4b
Copy link
Collaborator Author

@ekluzek This is ready to merge now with your review.

@ekluzek
Copy link
Collaborator

ekluzek commented Jan 29, 2025

OK, approved and ready to launch!

@jedwards4b jedwards4b merged commit bc29792 into ESCOMP:main Jan 29, 2025
4 checks passed
@jedwards4b jedwards4b deleted the all_errors_to_log branch January 29, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants