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.

Sign up for GitHub

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

Jump to bottom

Fix several incorrect argument ranks in calls to WRF I/O API routines #184

Merged
merged 2 commits into from
Nov 3, 2021
Merged

Fix several incorrect argument ranks in calls to WRF I/O API routines #184

merged 2 commits into from
Nov 3, 2021

Conversation

mgduda
Copy link
Collaborator

@mgduda mgduda commented Nov 2, 2021

This PR corrects the actual argument rank in several calls to WRF I/O API
routines for reading and writing domain time-independent attributes.

@mgduda
Fix incorrect argument rank in calls to ext_*_put_dom_ti_* in output_…
166c988 
…module.F

In calls to ext_{int,ncd,gr1}_put_dom_ti_{integer,real} in output_module.F, the
'Data' dummy argument is an array, but a scalar actual argument was provided.
This error was picked up by the GNU Fortran 11.1.0 compiler:

  output_module.f90:1733:41:

   1733 |                                          var_value, &
        |                                         1
  ......
   1761 |                                          var_value, &
        |                                         2
  Error: Rank mismatch between actual argument at (1) and actual argument at (2) (rank-1 and scalar)
  output_module.f90:1678:41:

   1678 |                                          var_value, &
        |                                         1
  ......
   1706 |                                          var_value, &
        |                                         2
  Error: Rank mismatch between actual argument at (1) and actual argument at (2) (rank-1 and scalar)

The fix adopted by this commit is to simply make the scalar var_value into a
temporary size-1 array with (/var_value/).
@mgduda
Fix incorrect argument rank in calls to ext_*_get_dom_ti_* in input_m…
03e35b6 
…odule.F

In calls to ext_{int,ncd,gr1}_get_dom_ti_{integer,real} in input_module.F, the
'Data' dummy argument is an array, but a scalar actual argument was provided.
This error was picked up by the GNU Fortran 11.1.0 compiler:

  input_module.f90:881:41:

    881 |                                          var_value, &
        |                                         1
  ......
    909 |                                          var_value, &
        |                                         2
  Error: Rank mismatch between actual argument at (1) and actual argument at (2) (rank-1 and scalar)
  input_module.f90:822:41:

    822 |                                          var_value, &
        |                                         1
  ......
    854 |                                          var_value, &
        |                                         2
  Error: Rank mismatch between actual argument at (1) and actual argument at (2) (rank-1 and scalar)

The fix adopted by this commit is to declare a local array for use as the actual
argument, and to copy the first element of this array to var_value after a
successful call to ext_{int,ncd,gr1}_get_dom_ti_{integer,real}.
@mgduda
Copy link
Collaborator Author

mgduda commented Nov 2, 2021 •
edited
Loading

This PR addresses build errors pointed out in Issue #163 and in Issue #180.

@mgduda mgduda requested a review from davegill November 2, 2021 01:02
@davegill
Copy link
Collaborator

davegill commented Nov 2, 2021

@mgduda
Michael,

Is there a reason why some of these arrays you went the direction of (/ var_value /) and some are var_value_arr?

I ask, because likely what you do here we will need to conventionally put into the WRF model, as we have the exact same complaints of argument mismatches with the newer GNU compilers.

@davegill
Copy link
Collaborator

davegill commented Nov 2, 2021

@mgduda
Michael,
OK, I got it now.
On "put", you force the input scalar to be an array.
On"get" you store the output variable into an array (which we later pull out index 1.

@mgduda
Copy link
Collaborator Author

mgduda commented Nov 2, 2021

@davegill The details of the changes are explained in the commit messages:
03e35b6
166c988

I didn't think the PR description needed to go into such detail, though.

@davegill
Copy link
Collaborator

davegill commented Nov 2, 2021

@mgduda
Michael,

I tested the following from inside the standard WRF-system docker container (GNU 9.3.1). This is expected to work since gnu 9 does not make mismatched variables a fatal error.

  1. The standard WPS code builds (master branch).
  2. This PR's branch (fix_ioapi_put_get_args) builds

I tested the following with GNU 11.1.0 on cheyenne, where I expected to see an impact.

  1. The standard WPS code does not build (master branch) for geogrid, ungrib, or metgrid.
  2. This PR's branch (fix_ioapi_put_get_args) builds geogrid.exe and metgrid.exe (what was expected).

@mgduda
Copy link
Collaborator Author

mgduda commented Nov 2, 2021

@davegill Once this PR has been merged, I was planning to create one more PR for the v4.3.1 release that adds -fallow-argument-mismatch to the GNU Fortran flags; that should allow everything to build, but I first wanted to address legitimate issues with subroutine arguments before papering over the remaining errors (e.g., calling MPI_Bcast with different argument types, which is completely fine and unavoidable).

@davegill
Copy link
Collaborator

davegill commented Nov 2, 2021

@mgduda
Michael,
I like your 2-PR idea.

@mgduda mgduda merged commit 244279e into wrf-model:release-v4.3.1 Nov 3, 2021
@mgduda mgduda deleted the fix_ioapi_put_get_args branch November 3, 2021 00:06
@mgduda mgduda mentioned this pull request Feb 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davegill davegill davegill approved these changes

Assignees
No one assigned
Labels
bug geogrid metgrid
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mgduda @davegill