Skip to content

Insert Compadre into Trilinos #164

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

Closed
7 tasks done
jmgate opened this issue Mar 2, 2020 · 14 comments
Closed
7 tasks done

Insert Compadre into Trilinos #164

jmgate opened this issue Mar 2, 2020 · 14 comments
Assignees

Comments

@jmgate
Copy link
Collaborator

jmgate commented Mar 2, 2020

This issue is intended to track the overall progress of getting Compadre into Trilinos. It will spawn sub-issues both here and in Trilinos itself.

Requirements

  1. Compadre will continue to live in its own repository (this one), which is where the development will happen.
  2. Compadre will be snapshotted into Trilinos whenever the Compadre developers desire.
  3. When snapshotting into Trilinos, the kokkos and python directories will be ignored.
  4. Compadre will build via its raw CMake build system when being built on its own, and via TriBITS when being built as part of Trilinos.

Sub-Issues

@jmgate
Copy link
Collaborator Author

jmgate commented Mar 2, 2020

@kuberry, any changes/additions you'd like to make to the requirements above?

Also, you may want to add @rmmilewi, @mperego, and myself to the repo for the sake of conversation.

@jmgate
Copy link
Collaborator Author

jmgate commented Mar 3, 2020

@kuberry, here are some questions we need to answer before we can come up with any kind of time estimate for this work:

  1. Are you okay with restructuring Compadre such that Compadre itself is contained within a compadre directory in the repository's root directory, and the other stuff (kokkos, python) stays in that root directory?
  2. Are you wanting the tests/examples to run via CTest within Trilinos?
  3. Are you wanting Compadre to be tested as part of Trilinos' pull request testing, or do you intend to handle testing on your own?
  4. Adding Compadre to Trilinos will require inserting a TriBITS build system alongside your current CMake build system. Once the infrastructure is in place, will you (or someone on your team) be the one maintaining both build systems, or are you expecting to have continued support from 1424 indefinitely?

@kuberry
Copy link
Collaborator

kuberry commented Mar 3, 2020

  1. Is there a good reason to do this? Adding kokkos and python folder to the .gitignore which can live in the Trilinos repo would effectively prevent these files from being tracked.
  2. Yes, I think if someone uses Trilinos_ENABLE_COMPADRE_TESTS or something like that, we should run the 30+ tests currently registered in CTest.
  3. I don't think that it is important for other people contributing to Trilinos to be sure that my tests pass before submitting. However, I would still like them included in the nightly tests (so that I can see if something breaks).
  4. I expect no support from 1424 to maintain the existing (nonTriBITS) portion of the build system. If a massive change is made in Trilinos (such as abandoning TriBITS) then I would hope to have 1424 support making adjustments, but not for minor changes to the TriBITS build system.

@mperego
Copy link

mperego commented Mar 3, 2020

I think Compadre should be tested as part of Trilinos PR testing (3.) This would reduce the work load for @kuberry or other Compadre maintainers. Imagine that some change in Kokkos (or in Kokkos Kernel in the future) breaks Compadre. If Compadre is not part of the PR testing it would be on @kuberry to fix it.

@jmgate jmgate self-assigned this Mar 3, 2020
@jmgate
Copy link
Collaborator Author

jmgate commented Mar 3, 2020

  1. Is there a good reason to do this?

TriBITS provides a snapshot-dir.py tool, which is the preferred method for snapshotting a directory into Trilinos. It works at the directory level, and would make the compadre directory in Trilinos/packages have the exact same contents as compadre itself. If we don't reorganize Compadre, then we'll have to write a snapshotting wrapper script around snapshot-dir.py. That's possible—it's just not Trilinos' preference.

@jmgate
Copy link
Collaborator Author

jmgate commented Mar 4, 2020

@kuberry, it looks like we have three possibilities for your build system.

  1. Switch to TriBITS: This is likely the best option in terms of maintainability because you would have a single build system. It would take some time to convert Compadre over to it, and would take some time for you to get up-to-speed with TriBITS, but it's likely well worth the effort. @bartlettroscoe tells me much of the logic and variables in your top-level CMakeLists.txt file is handled by TriBITS. This is the approach taken by, e.g., seacas and kokkos-kernels. This would require anyone building Compadre to have access to TriBITS. If you're looking to Trilinos as a distribution and adoption mechanism, then those users will have it already; if someone doesn't they can grab it from GitHub.
  2. TriBITS logic in current CMake files: Alternatively we could rewrite all of you CMakeLists.txt files with some logic on what to do if building in Trilinos and what to do otherwise. This is the approach taken by kokkos. A significant downside is it can become a bit of a maintenance nightmare. Any time you can something in one side of the if block, you need to remember to make the corresponding change in the other side.
  3. TriBITS and current CMake files side-by-side: A third approach is we could make minimal changes to your current CMakeLists.txt files (add a block in the main one, and a single line in all the rest), and then create additional CMake files for TriBITS alongside what you currently have. This is the recommendation of TriBITS itself if you're not going to just switch over to them. It has the same downsides in terms of maintenance as the option above. An upshot is there's a clear delineation between the raw CMake build system and the TriBITS one.

What's your preference?

@bartlettroscoe
Copy link

Just to be clear, I am not going to endorse any of these options long term. The best option for your code depends on several different factors. Here, I just want to make some clarifications ...

1. Switch to TriBITS ... This is the approach taken by, e.g., seacas and kokkos-kernels

SEACAS yes, KokkosKernls no. Kokkos and KokkosKernels use a very specialized CMake build system that is actually option 2. TriBITS logic in current CMake files

This would require anyone building Compadre to have access to TriBITS

Actually no, you can just snapshot TriBITS Core into Compadre so developers and user can just type cmake and that is it. That is what SEACAS does as you can see in:

3. TriBITS and current CMake files side-by-side

If I would give one recommendation it would be to start with this since Compadre is a small package and this would be easy to prototype. It is the same amount of work as option 1. Switch to TriBITS with zero risk. Then, if you are okay with TriBITS, then you can consider dropping the native CMake build system.

@kuberry
Copy link
Collaborator

kuberry commented Mar 5, 2020

#3 sounds like a good option to me. I have no issue with keeping two versions of CMake files, and to the extent that they are completely separate, I think it would be better.

@jmgate
Copy link
Collaborator Author

jmgate commented Mar 5, 2020

Just a heads-up, it sounds like we're looking at ~6 weeks of work or so. I'll keep you posted as we learn more.

@mperego
Copy link

mperego commented Mar 5, 2020

@jmgate are you working full time on this? I.e. are you expecting to charge ~240 hours for this?

@jmgate
Copy link
Collaborator Author

jmgate commented Mar 5, 2020

No, sorry for the confusion there. For time spent out of meetings, I can spend up to half my time on this, assuming other things don't catch fire. Actual time spent will probably be ~80 hours. Again, just a rough estimate.

@mperego
Copy link

mperego commented Mar 5, 2020

Ah OK, that's in the ball park of what I expected!

@jmgate
Copy link
Collaborator Author

jmgate commented Apr 22, 2020

Once #173 is merged, we're ready to finish out this work with the following tasks:

  • Create a branch off of develop in Trilinos.
  • Add Compadre to the list of packages with
    diff --git a/PackagesList.cmake b/PackagesList.cmake
    index 5c541a2..3801b0c 100644
    --- a/PackagesList.cmake
    +++ b/PackagesList.cmake
    @@ -64,6 +64,7 @@ TRIBITS_REPOSITORY_DEFINE_PACKAGES(
       Kokkos                packages/kokkos                   PT
       Teuchos               packages/teuchos                  PT
       KokkosKernels         packages/kokkos-kernels           PT
    +  Compadre              packages/compadre                 EX
       RTOp                  packages/rtop                     PT
       Sacado                packages/sacado                   PT
       MiniTensor            packages/minitensor               PT
  • Snapshot Compadre into Trilinos.
  • Push the branch.
  • Create a PR in Trilinos.
  • Wait for testing to pass, reviews, etc.
  • Merge the PR.

@kuberry
Copy link
Collaborator

kuberry commented Apr 22, 2020

Hi @jmgate, I just created a PR trilinos/Trilinos#7241 of the initial snapshot + adding Compadre to PackageList.cmake.

It won't allow me to mark a reviewer, labels, etc... only the PR description.

@jmgate jmgate closed this as completed Apr 28, 2020
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

No branches or pull requests

4 participants