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

remove ESMF_GridCompGetInternalState from use statement, not working… #449

Conversation

jedwards4b
Copy link
Collaborator

… with nag and apparently not needed

Description of changes

Remove the use ESMF_GridCompGetInternalState it is not needed and fails for the nag compiler.

Specific notes

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 Apr 2, 2024
@@ -1601,10 +1601,8 @@ subroutine set_aoflux_in_pointers(fldbun_a, fldbun_o, aoflux_in, lsize, xgrid, r
if (chkerr(rc,__LINE__,u_FILE_u)) return
call fldbun_getfldptr(fldbun_a, 'Sa_shum', aoflux_in%shum, xgrid=xgrid, rc=rc)
if (chkerr(rc,__LINE__,u_FILE_u)) return
if (add_gusts) then
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ekluzek I wish to point out this change required for nag because it doesn't like the unallocated pointer in the subroutine interface even though the pointer is never used. This is a case where the nag compiler is requiring changes we don't otherwise need to make.

Copy link
Member

Choose a reason for hiding this comment

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

I see arguments both ways here, but I lean slightly towards saying that this is an argument for keeping nag: I believe this is enforcing an aspect of the Fortran standard, so by fixing this we (1) make it more likely that our code won't be rejected by various other compilers that we don't currently test with, and (2) avoid the potential for undefined behavior in compilers that accept the code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My main argument for nag itself is that it's found legit subscript overflow problems that for whatever reasons other compilers haven't caught. If we can use different options to catch those problems I'm happy to move away from nag.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In terms of the code here, I see Jim's point that it's annoying here. But, if Bill is right that it's enforcing the standard I do side towards that. But, it also might be something that other compilers could find by using stricter options to enforce the standard?

@jedwards4b
Copy link
Collaborator Author

I ran and passed ERC_D_Ln9.T5_T5_mg37.QPC4.izumi_nag.cam-outfrq3s_usecase, due to the urgency of the request this code change was not reviewed prior to merging.

@jedwards4b jedwards4b merged commit d6dc571 into ESCOMP:main Apr 2, 2024
2 checks passed
@jedwards4b jedwards4b deleted the fix/remove_use_ESMF_GridCompGetInternalState branch April 2, 2024 23:01
Copy link
Member

@billsacks billsacks left a comment

Choose a reason for hiding this comment

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

Approving after the fact... thank you very much @jedwards4b !

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