-
Notifications
You must be signed in to change notification settings - Fork 1
api/reduce #7
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
Draft
maxwilliams94
wants to merge
7
commits into
fix_proposed_structure
Choose a base branch
from
api/REDUCE
base: fix_proposed_structure
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
api/reduce #7
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
e1af931
update gitignore
542a80d
Initial attempt at wrapping MPI_REDUCE using mpilib20_reduce_m module
d62b742
add module procedures to interface mpilib20_reduce
e219006
The root argument of mpilib20_reduce_<T>() generics is optional, allo…
ecd0bf7
PR changes
80a6f66
Merge branch 'fix_proposed_structure' into api/REDUCE
ebf7145
Remove non-existent file from src/CMakeLists.txt
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| !> Wrapping of the MPI_REDUCE methods | ||
| !> TODOs | ||
| !> Up to rank 3 arrays | ||
| !> Remaining types (parametized derived types for precisions?) | ||
| !> real(sp), real(dp), real(qp) | ||
| !> complex | ||
| !> Overload for MPI_IN_PLACE - optional logical could work | ||
| !> Testing | ||
| !> Refactor when root_id moved to mpi_env_type | ||
| module mpilib20_reduce_m | ||
|
|
||
| use mpilib20_init_finalise, only : mpi_env_type, root_id | ||
| use mpi_bindings, only : MPI_REDUCE, MPI_Op, & | ||
| MPI_INTEGER | ||
|
|
||
| implicit none | ||
|
|
||
| private | ||
|
|
||
| interface mpilib20_reduce | ||
| module procedure mpilib20_reduce_int_scalar | ||
| module procedure mpilib20_reduce_int_vec | ||
| end interface | ||
|
|
||
| public :: mpilib20_reduce | ||
|
|
||
| contains | ||
|
|
||
|
|
||
| !> Reduces values on all processes to a single value | ||
| !> on process defined by process_id | ||
| !> TODO usage examples | ||
| subroutine mpilib20_reduce_int_scalar(sendbuf, recvbuf, & | ||
| operation, mpi_env, & | ||
| process_id) | ||
| !> Variable containing set to be sent | ||
| type(MPI_INTEGER), intent(in) :: sendbuf | ||
| !> Variable to receive reduced set | ||
| type(MPI_INTEGER), intent(out) :: recvbuf | ||
| !> MPI Operation | ||
| type(MPI_Op), intent(in) :: operation | ||
| !> Instance of the MPI environment | ||
| type(mpi_env_type), intent(out) :: mpi_env | ||
| !> Rank of the process to receive the reduced set (override default) | ||
| integer, optional, intent(in) :: process_id | ||
| integer :: process | ||
|
|
||
| !> Overide root process if passed | ||
| if (present(process_id)) then | ||
| process = process_id | ||
| else | ||
| process = root_id | ||
| end if | ||
|
|
||
| call MPI_REDUCE(sendbuf, recvbuf, 1, MPI_INTEGER, operation, process, & | ||
| mpi_env%comm, mpi_env%ierror) | ||
|
|
||
| end subroutine mpilib20_reduce_int_scalar | ||
|
|
||
|
|
||
| !> Reduces values on all processes to a single value | ||
| !> on process defined by process_id | ||
| !> TODO usage examples | ||
| subroutine mpilib20_reduce_int_vec(sendbuf, recvbuf, & | ||
| operation, mpi_env, & | ||
| process_id) | ||
| !> Variable containing set to be sent | ||
| type(MPI_INTEGER), intent(in) :: sendbuf(:) | ||
| !> Variable to receive reduced set | ||
| type(MPI_INTEGER), intent(inout) :: recvbuf(:) | ||
| !> MPI Operation | ||
| type(MPI_Op), intent(in) :: operation | ||
| !> Instance of the MPI environment | ||
| type(mpi_env_type), intent(inout) :: mpi_env | ||
| !> Rank of the process to receive the reduced set (override default) | ||
| integer, optional, intent(in) :: process_id | ||
| integer :: process | ||
|
|
||
| !> Overide root process if passed | ||
| if (present(process_id)) then | ||
| process = process_id | ||
| else | ||
| process = root_id | ||
| end if | ||
|
|
||
| call assert(size(sendbuf) == size(recvbuf), "size(sendbuf) /= size(recvbuf)") | ||
|
|
||
| call MPI_REDUCE(sendbuf, recvbuf, size(recvbuf), MPI_INTEGER, operation, process, & | ||
| mpi_env%comm, mpi_env%ierror) | ||
|
|
||
| end subroutine mpilib20_reduce_int_vec | ||
|
|
||
|
|
||
| end module mpilib20_reduce_m | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we want to line-split (to keep lines below 90 characters(?)). Here I've split after argument 2 because otherwise one goes over 90, but then operation (potentially being removed soon anyway) and mpi_env are grouped. I've put optional arguments on their own line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, not going over 90 (unless a comment) sounds sensible. I often like to put inputs and outputs on different lines but if we retain a consistent arg ordering with MPI commands, then clearly we can't. In which case do whatever looks neat. Not a fan of putting optionals on their own line.
More broadly we should introduce fprettify and set it up how we like, then we can just run it to apply consistent formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll keep it neat without putting optional arguments on their own line.
Sounds good re fprettify. We should get a config file written for it relatively soon. Once I commit these PR changes I'll have a good and see what get produced.