Skip to content

Insert return calls right after offending call to subroutines if an error occurs#1141

Closed
matusmartini wants to merge 8 commits into
NCAR:mainfrom
matusmartini:return-on-error
Closed

Insert return calls right after offending call to subroutines if an error occurs#1141
matusmartini wants to merge 8 commits into
NCAR:mainfrom
matusmartini:return-on-error

Conversation

@matusmartini
Copy link
Copy Markdown
Contributor

@matusmartini matusmartini commented Jun 5, 2025

Description of Changes:

The issue: Some subroutines do not have return statements after an error occurs. Instead two variables errflg and errmsg are reset or re-initialized in subsequent calls, and if an error occurs it is not captured and the program continues without stopping. Please note that some calls already have if(errflg/=0) return in place.

This PR fixes an issue to return right after an error occurs. Subroutines that use errflg and errmsg should be checked for errflg and abort if necessary. Insert if(errflg/=0) return after such calls.

In addition the following is added:

  1. Remove unnecessary return statements at the end of subroutines. In one case, there were actually two return statements right after each other.
  2. Improve error messages and remove duplicate print statements if they are the same as the errmsg.
  3. Change “can not” to “cannot” and other minor additions.

UPDATE: Thanks to reviewers' feedback, this PR also includes cleanup of GFS_phys_time_vary.fv3.F90 to remove copy_error calls from one-thread region (accessing errmsg and errflg directly).

Tests Conducted:

None with the current head of the repo. Only quick tests with an older hash with NEPTUNE. We would like to adopt this ccpp error (errflg and errmsg) handling on our next upgrade but we need this PR (once reviewed and approved) to be merged first.

Dependencies:

None

@matusmartini
Copy link
Copy Markdown
Contributor Author

FYI @climbfuji and thank you for your input on these modifications

Comment thread physics/Interstitials/UFS_SCM_NEPTUNE/GFS_phys_time_vary.fv3.F90 Outdated
Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

This is a great cleanup, thank you!

I know that some people consider a return before the end of a program unit as good programming, I don't have a strong opinion. It's up to the codeowners of these files, but if we can have it consistent then that's nice, of course.

Comment thread physics/Radiation/radiation_gases.f
Comment thread physics/Radiation/radiation_gases.f
Comment thread physics/MP/Morrison_Gettelman/aer_cloud.F Outdated
@grantfirl
Copy link
Copy Markdown
Collaborator

@matusmartini Could you please merge the main branch into this branch?

@grantfirl
Copy link
Copy Markdown
Collaborator

@AnningCheng-NOAA Could you please review changes to the MG microphysics code here?

@grantfirl
Copy link
Copy Markdown
Collaborator

I've always wondered why there were return statements right before the end of a subroutine when the end of the subroutine returns anyway. Apparently, according to SO, this was once considered good practice? Seems strange to me.

Copy link
Copy Markdown
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

@matusmartini All of these changes seem fine to me. The only push back that I could imagine is if some developer wanted their scheme to complete even if there is an internal error. I don't know why someone would want to though (perhaps debugging reasons?).

@grantfirl
Copy link
Copy Markdown
Collaborator

FYI, this passes the SCM RTs in NCAR/ccpp-scm#596

@matusmartini
Copy link
Copy Markdown
Contributor Author

@matusmartini Could you please merge the main branch into this branch?

Done. No conflicts

@AnningCheng-NOAA
Copy link
Copy Markdown
Contributor

@AnningCheng-NOAA Could you please review changes to the MG microphysics code here?

looks fine to me.

@AnningCheng-NOAA
Copy link
Copy Markdown
Contributor

AnningCheng-NOAA commented Jun 24, 2025 via email

Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji 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 to me now

@matusmartini
Copy link
Copy Markdown
Contributor Author

@grantfirl I do not know what is the best course of actions. Feel free to let me know when this PR is good to go and I could merge main and resolve conflicts?

@grantfirl
Copy link
Copy Markdown
Collaborator

I've combined this into #1159, so I'm going to close it. There isn't anything left to do. All tests pass and all combined PRs have been approved, so will merge shortly.

@grantfirl
Copy link
Copy Markdown
Collaborator

Closing since combined with #1159

@grantfirl grantfirl closed this Sep 2, 2025
grantfirl added a commit that referenced this pull request Sep 3, 2025
@matusmartini matusmartini deleted the return-on-error branch February 28, 2026 23:42
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.

7 participants