-
Notifications
You must be signed in to change notification settings - Fork 563
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
STK: Don't use MPI_COMM_WORLD
#12167
STK: Don't use MPI_COMM_WORLD
#12167
Conversation
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
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.
Thanks for the proposed changes.
stk is developed in a separate repository and periodically snapshotted into trilinos, so we will need to make changes there to avoid having these get lost at the next snapshot.
We agree with the general premise that MPI_COMM_WORLD should not be used except in main and in functions that are equivalent to main such as gtest unit-tests.
We will prefer to implement some of these changes slightly differently, such as adding a mechanism to pass a communicator from a scope that already has one, rather than creating a global-comm facility.
What time-frame are you hoping to have MPI_COMM_WORLD usage eliminated in trilinos?
@alanw0, this is some contract work we are having done. They are hoping to complete this task by next week. :) If you can meet that timeline, it would be great, but not necessary. Otherwise, would you be able to get this in before the end of the FY (~Sep. 22) and the next release (Trilinos 15.0)? Another part of this effort is to have a non-blocking PR test to warn developers about adding back MPI_COMM_WORLD usage, as a few years ago we had a similar effort and since have had some regression. :( |
Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; This inspection will remain valid until a new commit to source branch is performed. |
@ccober6 Thanks Curt, these changes don't look too difficult for stk, we can probably have them in stk/sierra by next week. Getting the snapshot back to trilinos is sometimes unpredictable. But beating the next release should not be a problem. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-11.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-11.4.2-uvm-off
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-2021.3
Jenkins Parameters
Using Repos:
Pull Request Author: JacobDomagala |
Great! Thanks @alanw0! |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-11.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-11.4.2-uvm-off
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-2021.3
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
2 similar comments
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
@alanw0 Please confirm when you have made (the equivalent of) these changes to STK. We can close this PR at that time. |
@jwillenbring we have made most of them, still a couple of stragglers we should have in the next week. I agree it's probably safe to go ahead and close this. We'll have a new stk snapshot coming into trilinos soon also. |
@alanw0 Have these changes been made in the Trilinos repo? |
No not yet. I have the changes in a snapshot of stk that I'm trying to put into trilinos, but it hasn't made it through PR testing yet. We apparently have something wrong with one of our newer doc-test executables. I'll try to get that through soon. |
@jwillenbring these stk changes did go into trilinos/develop mid-late last week. |
@trilinos/stk
This PR is part of the initiative to phase out the use of
MPI_COMM_WORLD
in Trilinos.