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

Some fortran fixes #9259

Closed
wants to merge 1 commit into from
Closed

Conversation

awlauria
Copy link
Contributor

@awlauria awlauria commented Aug 17, 2021

fortran module: assumed size arg must match actual array arg of any rank

Here's an example code that fails to compile prior to this checkin:

     use mpi
     integer, contiguous, pointer :: requests(:, :)
     integer :: ierr
     allocate(requests(2,2))
     requests(1,1) = 1
     requests(2,1) = 2
     requests(1,2) = 3
     requests(2,2) = 4
     call MPI_Waitall(size(requests),requests,MPI_STATUSES_IGNORE,ierr)
     end

But according to the standard under "A.4 Fortran Bindings with mpif.h or the mpi
Module", waitall is supposed to take

    INTEGER COUNT, ARRAY_OF_REQUESTS(*)

and under fortran that assumed size arg should match the above actual arg.

The fortran module previously had

    integer, dimension(count), intent(inout) :: array_of_requests

and this checkin changes it to

    integer, dimension(*), intent(inout) :: array_of_requests

According to Rafik Zurob the rules for assumed-size arrays are such that we know
the incoming array_of_requests is contiguous so nothing changes in the fundamental
operation of the waitall call.

Signed-off-by: Austen Lauria [email protected]

@awlauria awlauria changed the title Fortran logical fix. Some fortran fixes Aug 17, 2021
@awlauria awlauria requested a review from jsquyres August 17, 2021 14:55
@awlauria
Copy link
Contributor Author

I will take a look at the mailmap faiure.

@awlauria
Copy link
Contributor Author

mailmap issue should be resolved.

@awlauria awlauria force-pushed the master_fort_fixes branch 4 times, most recently from 5c7e85e to 095a8c8 Compare August 21, 2021 12:20
Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

I think the corresponding changes are also needed in the use-mpi-ignore-tkr directory, too, right?

@awlauria
Copy link
Contributor Author

awlauria commented Sep 8, 2021

@markalle can you check to see if there's any additional changes needed?

@awlauria
Copy link
Contributor Author

@jsquyres I think @markalle addressed this in a seperate PR here:
#9367

Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

Upon further review, I do not believe that the 2nd commit on this PR is correct.

I'm fine with the first commit, but the 2nd one needs to be either removed or revised per #9367 (review).

Here's an example code that fails to compile prior to this checkin:
     use mpi
     integer, contiguous, pointer :: requests(:, :)
     integer :: ierr
     allocate(requests(2,2))
     requests(1,1) = 1
     requests(2,1) = 2
     requests(1,2) = 3
     requests(2,2) = 4
     call MPI_Waitall(size(requests),requests,MPI_STATUSES_IGNORE,ierr)
     end

But according to the standard under "A.4 Fortran Bindings with mpif.h or the mpi
Module", waitall is supposed to take
    INTEGER COUNT, ARRAY_OF_REQUESTS(*)
and under fortran that assumed size arg should match the above actual arg.

The fortran module previously had
    integer, dimension(count), intent(inout) :: array_of_requests
and this checkin changes it to
    integer, dimension(*), intent(inout) :: array_of_requests

According to Rafik Zurob the rules for assumed-size arrays are such that we know
the incoming array_of_requests is contiguous so nothing changes in the fundamental
operation of the waitall call.

Signed-off-by: Austen Lauria <[email protected]>

Co-authored-by: Jeff Squyres <[email protected]>
@awlauria
Copy link
Contributor Author

I seperated out the first commit to a seperate PR: #9405

And removed it from here.

@awlauria awlauria marked this pull request as draft September 22, 2021 14:27
@awlauria
Copy link
Contributor Author

I converted this to a draft until we decide what to do with it.

@awlauria awlauria closed this Oct 25, 2021
@awlauria awlauria deleted the master_fort_fixes branch October 25, 2021 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants