First iteration of HashmapAccumulator cleanup #731

e10harvey · 2020-05-29T21:47:16Z

The following changes were made to the HashmapAccumulator class:

max_value_size changed to private member __max_value_size.
hash_key_size was removed.
used_size was removed.
Hashes are now computed within the HashmapAccumulator insertion routines:
- __hashOpRHS was added as a private member.
- __compute_hash was added as a private member. This function is selected at compile time via a templated argument to the HashmapAccumulator constructor.
- vector_atomic_insert_into_hash_mergeAdd_with_team_level_list_length does not compute hashes internally due to the special use-case in KokkosSparse_spgemm_impl_speed.hpp:operator GPUTag.

Fixes #508.

spot-checks

1

<snip>
WARNING!! THE FOLLOWING CHANGES ARE UNCOMMITTED!! :
?? build/
?? testing/

KokkosKernels Repository Status:  7ff59541f124a9756cabd2c6949758500db58afe common/HashmapAccumulator: cleanup insert fn declarations more

Kokkos Repository Status:  33f730c475802bc226fc17e42e58fe7612d86b41 Merge pull request #3073 from masterleinad/enable_travis_compiler_warnings


Going to test compilers:  gcc/6.4.0 gcc/7.2.0 ibm/16.1.1 cuda/9.2.88 cuda/10.1.105
<snip>
#######################################################
PASSED TESTS
#######################################################
cuda-10.1.105-Cuda_OpenMP-release build_time=647 run_time=170
cuda-10.1.105-Cuda_Serial-release build_time=587 run_time=186
cuda-9.2.88-Cuda_OpenMP-release build_time=664 run_time=185
cuda-9.2.88-Cuda_Serial-release build_time=650 run_time=201
gcc-6.4.0-OpenMP_Serial-release build_time=228 run_time=139
gcc-7.2.0-OpenMP-release build_time=158 run_time=61
gcc-7.2.0-OpenMP_Serial-release build_time=220 run_time=138
gcc-7.2.0-Serial-release build_time=141 run_time=71
ibm-16.1.1-Serial-release build_time=1012 run_time=74

2

KokkosKernels Repository Status:  7ff59541f124a9756cabd2c6949758500db58afe common/HashmapAccumulator: cleanup insert fn declarations more

Kokkos Repository Status:  33f730c475802bc226fc17e42e58fe7612d86b41 Merge pull request #3073 from masterleinad/enable_travis_compiler_warnings


Going to test compilers:  cuda/9.2
Testing compiler cuda/9.2
  Starting job cuda-9.2-Cuda_OpenMP-release
kokkos devices: Cuda,OpenMP
kokkos arch: Kepler35
kokkos options: 
kokkos cuda options: force_uvm
kokkos cxxflags: -O3 -Wall -Wshadow -pedantic -Werror -Wsign-compare -Wtype-limits -Wuninitialized 
extra_args: 
kokkoskernels scalars: 'double,complex_double'
kokkoskernels ordinals: int
kokkoskernels offsets: int,size_t
kokkoskernels layouts: LayoutLeft
[eharvey@kokkos-dev testing]$ tail -f do-test-issue-508-05-29-2020.out 
kokkos devices: Cuda,OpenMP
kokkos arch: Kepler35
kokkos options: 
kokkos cuda options: force_uvm
kokkos cxxflags: -O3 -Wall -Wshadow -pedantic -Werror -Wsign-compare -Wtype-limits -Wuninitialized 
extra_args: 
kokkoskernels scalars: 'double,complex_double'
kokkoskernels ordinals: int
kokkoskernels offsets: int,size_t
kokkoskernels layouts: LayoutLeft
  PASSED cuda-9.2-Cuda_OpenMP-release
#######################################################
PASSED TESTS
#######################################################
cuda-9.2-Cuda_OpenMP-release build_time=1095 run_time=264

3

<snip>
WARNING!! THE FOLLOWING CHANGES ARE UNCOMMITTED!! :
?? build-issue-508/
?? build.750fe245/
?? build/
?? do-cmake.sh
?? do-test.sh
?? issue-727/
?? testing/

KokkosKernels Repository Status:  7ff59541f124a9756cabd2c6949758500db58afe common/HashmapAccumulator: cleanup insert fn declarations more

Kokkos Repository Status:  cb9727fae308ce7ae2248dbb8168c430d958bc32 core/src/impl: Conditionally define get_gpu in Kokkos_Core
<snip>
#######################################################
PASSED TESTS
#######################################################
clang-8.0-Cuda_OpenMP-release build_time=687 run_time=159
clang-8.0-Pthread_Serial-release build_time=207 run_time=110
clang-9.0.0-Pthread-release build_time=126 run_time=61
clang-9.0.0-Serial-release build_time=207 run_time=56
cuda-10.1-Cuda_OpenMP-release build_time=806 run_time=149
cuda-9.2-Cuda_Serial-release build_time=753 run_time=180
gcc-4.8.4-OpenMP-release build_time=126 run_time=58
gcc-7.3.0-OpenMP-release build_time=136 run_time=57
gcc-7.3.0-Pthread-release build_time=111 run_time=52
gcc-8.3.0-Serial-release build_time=135 run_time=58
gcc-9.1-OpenMP-release build_time=170 run_time=58
gcc-9.1-Serial-release build_time=155 run_time=62
intel-17.0.1-Serial-release build_time=262 run_time=65
intel-18.0.5-OpenMP-release build_time=338 run_time=53
intel-19.0.5-Pthread-release build_time=469 run_time=56
<snip>
WARNING!! THE FOLLOWING CHANGES ARE UNCOMMITTED!! :
?? build-issue-508/
?? build.750fe245/
?? build/
?? do-cmake.sh
?? do-test.sh
?? issue-727/
?? testing/

KokkosKernels Repository Status:  7ff59541f124a9756cabd2c6949758500db58afe common/HashmapAccumulator: cleanup insert fn declarations more

Kokkos Repository Status:  33f730c475802bc226fc17e42e58fe7612d86b41 Merge pull request #3073 from masterleinad/enable_travis_compiler_warnings


Going to test compilers:  gcc/7.3.0 gcc/8.3.0 gcc/9.1 gcc/4.8.4 intel/17.0.1 intel/18.0.5 intel/19.0.5 clang/8.0 clang/9.0.0 cuda/10.1
<snip>
#######################################################
PASSED TESTS
#######################################################
clang-8.0-Cuda_OpenMP-release build_time=606 run_time=153
clang-8.0-Pthread_Serial-release build_time=197 run_time=111
clang-9.0.0-Pthread-release build_time=115 run_time=53
clang-9.0.0-Serial-release build_time=126 run_time=57
cuda-10.1-Cuda_OpenMP-release build_time=837 run_time=151
gcc-4.8.4-OpenMP-release build_time=116 run_time=60
gcc-7.3.0-OpenMP-release build_time=138 run_time=59
gcc-7.3.0-Pthread-release build_time=110 run_time=53
gcc-8.3.0-Serial-release build_time=136 run_time=59
gcc-9.1-OpenMP-release build_time=169 run_time=60
gcc-9.1-Serial-release build_time=153 run_time=59
intel-17.0.1-Serial-release build_time=245 run_time=56
intel-18.0.5-OpenMP-release build_time=363 run_time=53
intel-19.0.5-Pthread-release build_time=442 run_time=47

When enabling all scalars

$ ./KokkosKernels_sparse_cuda --gtest_filter=cuda.sparse_spmv_* still results in:

[==========] 32 tests from 1 test case ran. (233294 ms total)
[  PASSED  ] 20 tests.
[  FAILED  ] 12 tests, listed below:
[  FAILED  ] cuda.sparse_spmv_float_int_int_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_struct_float_int_int_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_float_int_size_t_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_struct_float_int_size_t_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_kokkos_complex_float_int_int_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_struct_kokkos_complex_float_int_int_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_kokkos_complex_float_int_size_t_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_struct_kokkos_complex_float_int_size_t_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_mv_float_int_int_LayoutLeft_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_mv_float_int_size_t_LayoutLeft_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_mv_kokkos_complex_float_int_int_LayoutLeft_TestExecSpace
[  FAILED  ] cuda.sparse_spmv_mv_kokkos_complex_float_int_size_t_LayoutLeft_TestExecSpace

12 FAILED TESTS

and $ ./KokkosKernels_sparse_cuda --gtest_filter=cuda.sparse_spgemm_* still results in:

[==========] 16 tests from 1 test case ran. (127993 ms total)
[  PASSED  ] 12 tests.
[  FAILED  ] 4 tests, listed below:
[  FAILED  ] cuda.sparse_spgemm_jacobi_float_int_int_TestExecSpace
[  FAILED  ] cuda.sparse_spgemm_jacobi_float_int_size_t_TestExecSpace
[  FAILED  ] cuda.sparse_spgemm_jacobi_kokkos_complex_float_int_int_TestExecSpace
[  FAILED  ] cuda.sparse_spgemm_jacobi_kokkos_complex_float_int_size_t_TestExecSpace

 4 FAILED TESTS

- Remove max_value_size as parameter to insertion routines. - Add vector_atomic_insert_into_hash_mergeAdd_with_team_level_list_length to support spgemm "speed" use-case with team-level list lengths.

…sert_into_hash_TrackHashes

…sert_into_hash_mergeOr_TrackHashes

- Remove unused params from vector_atomic_insert_into_hash_mergeOr - Conditionally assert and abort upon insertion of hash = -1 - Add comments

…ines

seheracer · 2020-06-01T15:48:30Z

Which machine are these spot-check results from?

brian-kelley

Generally everything looks great, there's just a couple minor changes I would like to see

src/common/KokkosKernels_HashmapAccumulator.hpp

src/sparse/impl/KokkosSparse_spgemm_impl_compression.hpp

src/sparse/impl/KokkosSparse_spgemm_impl_symbolic.hpp

src/sparse/impl/KokkosSparse_spgemm_impl_triangle.hpp

src/common/KokkosKernels_HashmapAccumulator.hpp

e10harvey · 2020-07-07T19:11:17Z

@ndellingwood, @srajama1: This is ready to merge.

ndellingwood · 2020-07-08T18:01:26Z

Thanks @e10harvey !

e10harvey added 20 commits May 20, 2020 11:37

common/HashmapAccumulator: Remove used_size member

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

2011e81

common/HashmapAccumulator: remove hash_key_size

30a3c98

common/HashmapAccumulator: make __INSERT_{FULL,SUCCESS} private members

Loading
Loading status checks…

63374fe

common/HashmapAccumulator: start adding internal compute_hash

fc207b2

common/HashmapAccumulator: use __compute_hash routine

5350c60

sparse/impl: remove unused variables

e44cfa6

common/HashmapAccumulator: Remove unused params from vector_atomic_in…

6548fd3

…sert_into_hash_TrackHashes

common/HashmapAccumulator: Remove unused params from vector_atomic_in…

1e2b678

…sert_into_hash_mergeOr_TrackHashes

unit_test/sparse: is_same_matrix refactor variable names

7b25785

common/HashmapAccumulator:

6e3cb61

- Remove unused params from vector_atomic_insert_into_hash_mergeOr - Conditionally assert and abort upon insertion of hash = -1 - Add comments

common/HashmapAccumulator: Remove hash parameter from all insert rout…

4c9a815

…ines

src/sparse/impl: jacobi sparseacc remove unused var

89031ee

src/sparse/impl: Fix unsuccess count in GpuTag for spgemm compression

d3d824c

common/HashmapAccumulator: Assert key != -1

2a59230

common/HashmapAccumulator: Return if key = -1

a780b18

common/HashmapAccumulator: cleanup insert fn declarations

29837f5

common/HashmapAccumulator: cleanup insert fn declarations more

7ff5954

common/HashmapAccumulator: Disable asserts

249e308

common/HashmapAccumulator: Cleanup comments

ebab3bc

e10harvey added the Cleanup label May 29, 2020

e10harvey requested review from brian-kelley, srajama1, ndellingwood and seheracer May 29, 2020 21:47

e10harvey self-assigned this May 29, 2020

brian-kelley requested changes Jun 9, 2020

View reviewed changes

Implement PR feedback

e075e61

e10harvey force-pushed the issue-508 branch from 3e97f72 to e075e61 Compare June 15, 2020 21:10

common/HashmapAccumulator: Update comments

60d1236

e10harvey commented Jun 16, 2020

View reviewed changes

src/common/KokkosKernels_HashmapAccumulator.hpp Show resolved Hide resolved

e10harvey force-pushed the issue-508 branch from 3f648d6 to 60d1236 Compare June 17, 2020 13:05

common/HashmapAccumulator: Update comments

9a92a5a

e10harvey requested a review from brian-kelley July 7, 2020 19:06

brian-kelley approved these changes Jul 7, 2020

View reviewed changes

ndellingwood merged commit f89a3d9 into kokkos:develop Jul 8, 2020

e10harvey linked an issue Jul 9, 2020 that may be closed by this pull request

HashmapAccumulator has several unused members, misnamed parameters #508

Closed

e10harvey deleted the issue-508 branch November 14, 2020 13:12

kokkos-devops-admin mentioned this pull request Sep 8, 2024

ODE - RK: fixing small issues reported by Yaro #2229

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First iteration of HashmapAccumulator cleanup #731

First iteration of HashmapAccumulator cleanup #731

e10harvey commented May 29, 2020 •

edited

Loading

seheracer commented Jun 1, 2020

brian-kelley left a comment

e10harvey commented Jul 7, 2020

ndellingwood commented Jul 8, 2020

First iteration of HashmapAccumulator cleanup #731

First iteration of HashmapAccumulator cleanup #731

Conversation

e10harvey commented May 29, 2020 • edited Loading

spot-checks

1

2

3

When enabling all scalars

seheracer commented Jun 1, 2020

brian-kelley left a comment

Choose a reason for hiding this comment

e10harvey commented Jul 7, 2020

ndellingwood commented Jul 8, 2020

e10harvey commented May 29, 2020 •

edited

Loading