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

inline post restart issue with gnu compiler #321

Merged
merged 4 commits into from
Jun 2, 2021

Conversation

junwang-noaa
Copy link
Collaborator

@junwang-noaa junwang-noaa commented May 28, 2021

Description

This PR is to fix the issue with inline post in restart runs with gnu compiler.
Is a change of answers expected from this PR? The fix does not changes results.

Issue(s) addressed

Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues should always be created before starting work on a PR branch!)

  • fixes noaa-emc/fv3atm/issues/<320>

Testing

How were these changes tested?
Run the control restart test with gnu compiler

What compilers / HPCs was it tested with? GNU
Are the changes covered by regression tests?
NO, the current control restart does not have inline post. The global tests update (ufs-weather-model PR#561) has inline post turned on in the control and restart.
Have the ufs-weather-model regression test been run? On what platform?
Tested on hera. Will test on other tier-1 platforms

  • Will the code updates change regression test baseline?
    This change does not change results, but the ufs-weather-model PR#561 does require new baseline.

  • Please commit the regression test log files in your ufs-weather-model branch

Dependencies

If testing this branch requires non-default branches in other repositories, list them.
Those branches should have matching names (ideally)

Do PRs in upstream repositories need to be merged first?
If so add the "waiting for other repos" label and list the upstream PRs

io/post_gfs.F90 Outdated
@@ -156,11 +156,13 @@ subroutine post_run_gfs(wrt_int_state,mypei,mpicomp,lead_write, &
if(mype==0) print *,'af read_xml at fh00,name=',trim(filenameflat)
else if(ifhr > 0) then
filenameflat = 'postxconfig-NT.txt'
if(size(paramset)>0) then
if(associated(paramset) .and. size(paramset)>0) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Fortran compiler specifications do not state that the tests in an if clause will be executed ini the order written. You need to do the following to be fully compliant:

if (associated(paramset)) then
  if (size(paramset)>0) then

Copy link
Collaborator Author

@junwang-noaa junwang-noaa Jun 1, 2021

Choose a reason for hiding this comment

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

Actually the size(paramset) is 1 but associated(paramset) is false with gnu compiler.

@@ -160,11 +160,13 @@ subroutine post_run_regional(wrt_int_state,mypei,mpicomp,lead_write, &
if(mype==0) print *,'af read_xml at fh00,name=',trim(filenameflat)
else if(ifhr > 0) then
filenameflat = 'postxconfig-NT.txt'
if(size(paramset)>0) then
if(associated(paramset) .and. size(paramset)>0) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

Copy link
Collaborator

Choose a reason for hiding this comment

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

How? If associated(paramset) (means it's a pointer) is false, how can the size be 1?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is what I got when I tried with gnu compiler. The issue happened when it tried to get the size of paramset(1)%param.

I think it would be best to make the same changes as in lines 161-162, 165-166.

@junwang-noaa
Copy link
Collaborator Author

junwang-noaa commented Jun 1, 2021 via email

Copy link
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.

Thanks!

@junwang-noaa junwang-noaa merged commit 116d701 into NOAA-EMC:develop Jun 2, 2021
@junwang-noaa junwang-noaa deleted the gnu_contrlrst branch June 2, 2021 16:06
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