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

Add nompi stub #425

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open

Add nompi stub #425

wants to merge 15 commits into from

Conversation

bobpaw
Copy link
Collaborator

@bobpaw bobpaw commented Apr 26, 2024

Add nompi stub

Add a stub implementation of MPI in pcu/pcu_pnompi.c.
Remove direct calls to MPI functions.

This adds a SCOREC_NO_MPI build option for users running on single-process, single-machine setups that do not want to compile with MPI. Message passing is simulated in memory.

@bobpaw
Copy link
Collaborator Author

bobpaw commented Apr 26, 2024

@cwsmith, build is failing for this PR. I think we should add PCU_Init and PCU_Finalize facades that wrap the MPI calls and do a PCU_Comm_Init() and PCU_Comm_Free() to remove the direct dependencies. There are a lot of executables that would need to be changed and this would become a large PR. Alternatively, we could define a replacement MPI_Init and MPI_Finalize in pcu_pnompi and link to those. Or, we could do a hybrid approach and encourage people to use PCU_Init in the future.

@jacobmerson
Copy link
Contributor

@flagdanger and I are working on a very large update of PCU that should make this all easier by getting rid of the singleton. Pull request for that coming within the next week.

@jacobmerson
Copy link
Contributor

@bobpaw sorting out a few things to make the pull request ready to merge, but #388 the draft pull request for replacing the singleton in PCU.

Although we replaced the interface used in the test cases, this pull request is designed to work with no modification to existing codes by using a global PCU state when the old style interface is used with PCU_Comm_Init or it can use local state.

@bobpaw
Copy link
Collaborator Author

bobpaw commented Apr 30, 2024

@jacobmerson, we will plan to use your PR for the future of core, but the sponsor wants this functionality soon, so we will use this branch until yours is finished.

@cwsmith
Copy link
Contributor

cwsmith commented Apr 30, 2024

@bobpaw Does PCU_Comm_Init() check if MPI was initialized?

@bobpaw
Copy link
Collaborator Author

bobpaw commented Apr 30, 2024

@bobpaw Does PCU_Comm_Init() check if MPI was initialized?

No, at the moment it only checks if PCU has been initialized using a global state variable. We could check with MPI_Initialized.

@onkarsahni
Copy link
Contributor

@cwsmith Note Aiden is only doing this when SCOREC_NO_MPI is defined to be true, so there is no actual MPI (e.g., see pcu/pcu_pnompi.c). For now, he is not looking into PCU_init, that will be done after the other PR #388.

@cwsmith
Copy link
Contributor

cwsmith commented Apr 30, 2024

@bobpaw OK. I'm wondering if it will make sense to make the MPI check in that call, and if it fails call MPI_Init. But... handling MPI_Finalize could be a bit more tricky; other libraries may still be running and using MPI beyond the lifetime of PCU. I'd prefer symmetry in the interface so your original suggestion may be best.

@bobpaw bobpaw marked this pull request as ready for review May 6, 2024 15:13
Added pcu_pnompi.c.
Added SCOREC_NO_MPI CMake option.
Removed direct MPI calls in parma, phasta, and PUMI.
Added incomplete reimplementations of those MPI calls.
Add stubs for MPI_Init and MPI_Finalize.
Add pcu_pnompi_types.h for MPI typedefs.
Add status message about SCOREC_NO_MPI flag.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Replace MPI_Comm_free with PCU_Comm_Free_One.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
This will avoid object collisions when linking to libraries that already use MPI.
- i.e. the intended use case for SCOREC_NO_MPI.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Changed add_nompi_msg to record receiver.
- Since there's no MPI, sender == receiver but I want to quiet the warnings.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Add return type for pcu_pmpi_free, pcu_pmpi_split.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Add linked list traversal to find a matching message.
Removed error message that did not actually indicate errors.
- If no message is found it's perfectly reasonable and in fact correct to
  return 0 and that's how pmpi knows to return false.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Run smoke tests without mpirun given SCOREC_NO_MPI.
Add SCOREC_NO_MPI to the GitHub Actions test matrix.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
I use clock_gettime, which is one of the many methods mpich uses depending on
the system.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
Also update TriBITS package file.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
- Add mpi_test_depends function to replace set_test_properties. This
  function checks if SCOREC_NO_MPI is defined and assumes nonexistent
  tests were multiproc ones.

Signed-off-by: Aiden Woodruff <woodra@rpi.edu>
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.

4 participants