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

MueLu: Merge *_kokkos factories and their classical counterparts #11658

Open
22 of 36 tasks
cgcgcg opened this issue Mar 10, 2023 · 7 comments
Open
22 of 36 tasks

MueLu: Merge *_kokkos factories and their classical counterparts #11658

cgcgcg opened this issue Mar 10, 2023 · 7 comments
Labels
pkg: MueLu type: enhancement Issue is an enhancement, not a bug

Comments

@cgcgcg
Copy link
Contributor

cgcgcg commented Mar 10, 2023

@trilinos/muelu

Enhancement

We duplicated a lot of algorithms and code paths for use without and with Kokkos. Since MueLu effectively requires Kokkos now, we can start to remove these code duplications. Some removals are quite safe, but others will require a lot of work since the two code branches have diverged significantly.

NOTE: #11682 should be resolved before tackling anything involving coupled aggregation. Removed in #11687

For cases where the two paths are significantly different, one possible first step is to merge the files and based on the value of use kokkos refactor to take one code path or the other. This would allow to reduce the amount of code duplication. This approach does not work when the factories have outright different inputs and outputs.

The following classes have been merged, but still need to be rewritten to make sure that non-Kokkos and Kokkos code path have the same functionality:

  • AggregationPhase1Algorithm
  • AggregationPhase2aAlgorithm
  • AggregationPhase2bAlgorithm
  • AggregationPhase3Algorithm
  • OnePtAggregationAlgorithm
  • PreserveDirichletAggregationAlgorithm
  • SaPFactory - enforcement of constraints
@cgcgcg cgcgcg added type: enhancement Issue is an enhancement, not a bug pkg: MueLu labels Mar 10, 2023
@GrahamBenHarper
Copy link
Contributor

I can rename the matrix-free files since I made them.

@GrahamBenHarper
Copy link
Contributor

Also Aggregates_kokkos should be relatively easy to merge since I think I sync'd up their implemented methods with Aggregates mostly in #10902. The hard part will be merging all the uses of them.

@trilinos trilinos deleted a comment from github-actions bot Mar 10, 2023
@trilinos trilinos deleted a comment from github-actions bot Mar 10, 2023
@trilinos trilinos deleted a comment from github-actions bot Mar 10, 2023
@cgcgcg
Copy link
Contributor Author

cgcgcg commented Mar 10, 2023

@GrahamBenHarper For Aggregates_kokkos, yes, in principle it could just be a matter of accessing a view instead of a Vector. Are you volunteering?

@GrahamBenHarper
Copy link
Contributor

My schedule's tight right now with a deadline on this coming Tuesday, but after that I should definitely be able to find some time :)

@vbrunini
Copy link
Contributor

We've got a few tests failing in Trilinos integration builds because the factory names are used in XML files and the CoarseMapFactory_kokkos name is no longer recognized. Could we get a deprecation period where both the base factory names and the _kokkos versions are recognized in XML input?

@cgcgcg
Copy link
Contributor Author

cgcgcg commented Apr 12, 2023

@vbrunini That should be easy to do. I'll get back to you.

@cgcgcg
Copy link
Contributor Author

cgcgcg commented Apr 13, 2023

@vbrunini Could you let me know if #11770 helped with your integration builds?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: MueLu type: enhancement Issue is an enhancement, not a bug
Projects
None yet
Development

No branches or pull requests

3 participants