Skip to content

Resolve "Create TriBITS Build Alongside Raw CMake Build" #173

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

Merged
merged 32 commits into from
Apr 22, 2020
Merged

Resolve "Create TriBITS Build Alongside Raw CMake Build" #173

merged 32 commits into from
Apr 22, 2020

Conversation

jmgate
Copy link
Collaborator

@jmgate jmgate commented Mar 9, 2020

This is a WIP PR to address #171. Not ready for review or merge.

@jmgate jmgate added the don't merge Some problem with this request still needs addressed label Mar 9, 2020
@jmgate jmgate self-assigned this Mar 9, 2020
@kuberry
Copy link
Collaborator

kuberry commented Mar 10, 2020

Hi @jmgate, we have a condition where either CUSOLVER+CUBLAS will be needed, BLAS+LAPACK, or MKL. All of these are optional, but at least one of these sets of tools must be available. I think we need to verify the user has some combination that will work, but we can't specify what combination that would be (although for Cuda, it does imply CUBLAS).

@jmgate jmgate marked this pull request as draft April 16, 2020 23:02
@jmgate jmgate changed the title WIP: Resolve "Create TriBITS Build Alongside Raw CMake Build" Resolve "Create TriBITS Build Alongside Raw CMake Build" Apr 16, 2020
@jmgate jmgate marked this pull request as ready for review April 20, 2020 22:08
@jmgate jmgate requested a review from kuberry April 20, 2020 22:08
@jmgate jmgate removed the don't merge Some problem with this request still needs addressed label Apr 20, 2020
@jmgate
Copy link
Collaborator Author

jmgate commented Apr 20, 2020

@kuberry, this is ready for you to play around with. To test things out, I'd:

  1. Merge Resolve "Create Script to Snapshot Compadre into Trilinos" #176 into master.
  2. Rebase this branch on top of master.
  3. Create a branch off of develop in Trilinos.
  4. mkdir -p packages/compadre.
  5. In the Compadre root, ./scripts/snapshot_into_trilinos.py --trilinos-dir /path/to/Trilinos.
  6. Create a build directory.
  7. Modify the trilinos-config.sh to fill in the appropriate paths.
  8. Run that script, make, and ctest.
  9. Adjust settings in CMakeLists.txt until the tests pass.

Let me know when things look good on your end, because there's one more file we need to tweak in Trilinos when we do the initial snapshot.

@kuberry
Copy link
Collaborator

kuberry commented Apr 21, 2020

Hi @jmgate, I'm just going to drop a list here of things we discussed:

  • Ability to move files from copied location in compadre folder in Trilinos/packages and create an additional commit to that effect during snapshotting
  • Dealing with either LAPACK+BLAS, or MKL, or CUBLAS+CUSOLVER (as long as 1 of 3 is available)
  • Mpiexec shouldn't be called to run examples. All examples/tests in repo are single processor (OpenMP for multicore) [This is the reason tests are failing]
  • I will try to get Compadre_Config.h.in to be produced automatically
  • I will put glob in for src header/source files
  • I will make Version.cmake to be version used by CMakeLists.txt for raw CMake build

I'll add more as I think of them.

@jmgate
Copy link
Collaborator Author

jmgate commented Apr 21, 2020

For the mpiexec bit, with tribits_add_test() you can specify the number of cores to use with NUM_MPI_PROCS 1.

BTW, I just rebased this on top of master so it's all up-to-date.

@kuberry
Copy link
Collaborator

kuberry commented Apr 21, 2020

If the only two files needed in the root of the repo are Version.cmake and TPLLists.cmake, then I'll mark "Ability to move files from copied location in compadre folder in Trilinos/packages and create an additional commit to that effect during snapshotting" as done. I'm going to reuse Version.cmake, so it can stay there. For TPLLists.cmake, I'll probably add a commented out section in there later explaining the dependencies of the package and how the code is for Trilinos, but how a user can specify these needed things if they using a raw CMake build.

both mpi and serial for COMM.

Changed python based tests to use PYTHON_EXECUTABLE and to be created
as an advanced tests which allows for avoiding the call to python by
mpiexec.

Compadre_Config.h is now produced using bob features for both raw
CMake and Tribits build. Modified bob call to allow for a module name
rather than it being hard coded for ${PROJECT_NAME}.
@kuberry
Copy link
Collaborator

kuberry commented Apr 21, 2020

Most recent commit eb4ecec should pass all tests for MPI or serial build of package in Trilinos.

kuberry added 3 commits April 21, 2020 17:12
Added flags --porder and --grids to Python examples for raw CMake tests.
so removed as these are already defined in Trilinos/packages root.

Version is now defined at the top of CMakeLists.txt

Logic for in-Trilinos build set to:
Unless specified, Compadre_USE_LAPACK=OFF, Compadre_USE_MKL=OFF,
Compadre_USE_CUDA=OFF.

This allows enabling any one of the three options explicitly to not
interfere with the others (setting Compadre_USE_LAPACK=ON by default
would interfere with setting Compadre_USE_MKL=ON as only setting)

cmake/Dependencies.cmake now calls for different required packages
dependent on Compadre_USE_LAPACK, Compadre_USE_MKL, and Compadre_USE_CUDA
@kuberry
Copy link
Collaborator

kuberry commented Apr 22, 2020

Commit 54ab2b2 handles Compadre_USE_LAPACK, Compadre_USE_MKL, and Compadre_USE_CUDA options (Compadre_USE_LAPACK is default) and will ensure that the correct TPLs are turned on or else will cause Compadre package in Trilinos to be disabled.

…ls isn't

a required package for Compadre in Trilinos.

Removed TPL_ENABLE_LAPACK and TPL_ENABLE_BLAS from trilinos-configure.sh
because these are now enabled automatically by cmake/Dependencies.cmake.
Copy link
Collaborator

@kuberry kuberry left a comment

Choose a reason for hiding this comment

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

@jmgate, I'm good with the way this is behaving and the current PR solves all of the checkbox of issues outlined above.

@jmgate
Copy link
Collaborator Author

jmgate commented Apr 22, 2020

Yup, this all looks good on my system as well. Thanks for figuring out the last few hurdles, @kuberry. Merge if you're happy with it.

@kuberry
Copy link
Collaborator

kuberry commented Apr 22, 2020

Thanks @jmgate for all of your hard work putting this together!

@kuberry kuberry merged commit 5d76dd6 into sandialabs:master Apr 22, 2020
@jmgate jmgate mentioned this pull request Apr 22, 2020
7 tasks
@jmgate jmgate deleted the 171-create-tribits-build-alongside-raw-cmake-build branch April 22, 2020 14:44
@jmgate jmgate mentioned this pull request May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants