-
Notifications
You must be signed in to change notification settings - Fork 557
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
Tpetra: use KokkosKernels addition (and remove redundant copies of these kernels in Tpetra) #2444
Conversation
Using KokkosKernels sparse matrix addition kernels in TpetraExt::MatrixMatrix::add(). Removed the now redundant copies of these kernels in Tpetra.
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Using Repos:
Pull Request Author: brian-kelley |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
@brian-kelley Thanks for working on this! Could you please give me some context for the code that you added? |
That is, I can see that you removed code, but you also added code; I'm asking about the latter :-) . |
@mhoemmen I didn't want to change anything in TpetraExt::MatrixMatrix::add(), so add() still calls one of 3 internal functions in AddDetails::AddKernels: addSorted(), addUnsorted(), and convertToGlobalAndAdd(). I replaced the internals of these with code to make KK handles and call spadd_symbolic and spadd_numeric. Also the TimeMonitor just measures symbolic and numeric now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! I think it's OK. Does it pass CUDA tests? Does it require some experimental kokkos-kernels macros?
@@ -138,6 +138,7 @@ struct AddKernels | |||
typedef Tpetra::Map<LocalOrdinal, GlobalOrdinal, Node> map_type; | |||
typedef typename Node::device_type device_type; | |||
typedef typename device_type::execution_space execution_space; | |||
typedef typename execution_space::memory_space memory_space; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this build clean with -Wall
? Lots of typedefs make me nervous sometimes ;-) .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
@@ -561,7 +562,6 @@ makeColMapAndConvertGids(GlobalOrdinal ncols, | |||
return rcp(new map_type(Teuchos::OrdinalTraits<GlobalOrdinal>::invalid(), colmap, 0, comm)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically should be Teuchos::OrdinalTraits<Tpetra::global_size_t>::invalid()
but that's OK :)
ColInds Ccolinds; | ||
}; | ||
using Teuchos::TimeMonitor; | ||
TEUCHOS_TEST_FOR_EXCEPTION(Arowptrs.dimension_0() != Browptrs.dimension_0(), std::runtime_error, "Can't add matrices with different numbers of rows."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a new test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or was it there before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was there before. Git thinks it's new because the indentation changed for this function.
@brian-kelley Once this is ready for pull request testing, be sure to remove the "AT: WIP" label. Thanks! |
@brian-kelley The title matters less than the "AT: WIP" label; I just removed the latter. |
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ mhoemmen ]! |
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - Master Automerge is disabled (in .cfg file) |
I added the retest label to force a rebase and retest, since it's been a few days. |
Status Flag 'Pull Request AutoTester' - User Requested Retest - Resetting Testing Status |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Using Repos:
Pull Request Author: brian-kelley |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
|
Status Flag 'Pull Request AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - Master Automerge is disabled (in .cfg file) |
…evelop * 'develop' of https://github.com/trilinos/Trilinos: (560 commits) Disabling Stefan Boltzmann tests 1 and 2 due to an unresolved hang. Also, resetting the default problem size for Helmholtz to 16x16. Disable 3 Anasazi tests that randomly fail in debug builds on white/ride (trilinos#2473) TrilinosCouplings: Output iteration count Tpetra: use KokkosKernels addition (trilinos#2444) Tank solve and value correspond for all parameters TrilinosCouplings: OK. Now compiling TrilinosCouplings: More RTC updates Disabling failing test. Stokhos: Allow mean-based PCE preconditioner with double scalar type. (Painstakingly) reimplemented every tank equation individually. Now have solve and value working correctly together. TrilinosCouplings: Turning off file default output Kokkos: fix compilation for GCC 4.8.4 TrilinosCouplings: Adding block / RTC materials support to Tpetra example (take2) Kokkos: disable failing CUDA+DEBUG test TrilinosCouplings: Adding block / RTC materials support to Tpetra example adding doxygen for nd method Added comment Fixed warnings. Panzer: fix race condition in unit test exodus writer for CurlLaplacian example Fixed some problems in tank example. Solve and value are at least consistent when theta=1 ...
Using KokkosKernels sparse matrix addition kernels in
TpetraExt::MatrixMatrix::add(). Removed the now redundant
copies of these kernels in Tpetra.
@trilinos/tpetra
Description
Motivation and Context
How Has This Been Tested?
Screenshots
Checklist
Additional Information