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

Testing: Address case when setting COMM serial and NUM_MPI_PROCS N for N > 1 #542

Open
bartlettroscoe opened this issue Dec 1, 2022 · 3 comments

Comments

@bartlettroscoe
Copy link
Member

bartlettroscoe commented Dec 1, 2022

Description

As described in trilinos/Trilinos#11322 (comment), the updated behavior of TriBITS is to not add a test in an non-MPI SERIAL build when the test has an explicit NUM_MPI_PROCS N for N > 1. But test will never be added if COMM serial is passed into tribits_add[_advanced]_test() in these cases.

This Issue is to update tribits_add[_advanced]_test() to address the case where COMM serial and NUM_MPI_PROCS N for N > 1 are passed in.

Proposed solution

Many different proposed solutions are discussed in comments after trilinos/Trilinos#11322 (comment). What we arrived at is to implement the following:

Case 1: NUM_MPI_PROCS N for N > 1 but no explicit COMM ... argument passed in:

  • MPI builds: Test will be added with N procs (which is the current TriBITS behavior)
  • Serial builds: Test will be slipped (which is the current TriBITS behavior)

Case 2: NUM_MPI_PROCS N for N > 1 with explicit COMM serial mpi passed in:

  • MPI builds: Two tests will be added: one with N procs and one with 1 proc (currently just the N proc test is added)
  • Serial builds: One test with 1 proc will be added (currently no test is added)

Case 3: NUM_MPI_PROCS N for N > 1 with explicit COMM serial (but not mpi) passed in:

  • Configure error for MPI and Serial builds with error message stating that the test will never be added and provide suggestions for how to fix this to do something logical (currently the test is skipped with a message why printed to the script when tracing add_test)
@bartlettroscoe bartlettroscoe changed the title Testing: Error out when setting COMM serial and NUM_MPI_PROCS N for N > 1 Testing: Address case when setting COMM serial and NUM_MPI_PROCS N for N > 1 Dec 1, 2022
@bartlettroscoe
Copy link
Member Author

@rppawlo and @etphipp, can you please look above and add a thumbs up if those behaviors are consistent with what we have agreed to in trilinos/Trilinos#11322 (comment)? If not, please comment on what should be fixed.

@bartlettroscoe
Copy link
Member Author

@rppawlo and @etphipp, now the question is, how urgent is this compared to #367? But this is something I might get help with from Kitware.

@etphipp
Copy link

etphipp commented Dec 5, 2022

I can't speak to the urgency #367, but I think this is somewhat urgent just because we have a bunch of tests that aren't being run in serial builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Selected
Development

No branches or pull requests

2 participants