From b72c6eb0b5d7bfb7ae6f9e5dd975fee053f401df Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 14:11:00 +0200 Subject: [PATCH 01/11] Set v0.6.0 in build system --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cb5f342a1..a7dc72dd2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -27,7 +27,7 @@ cmake_minimum_required( VERSION 3.13 ) set( MAJORVERSION 0 ) -set( MINORVERSION 5 ) +set( MINORVERSION 6 ) set( BUGVERSION 0 ) set( VERSION "${MAJORVERSION}.${MINORVERSION}.${BUGVERSION}" ) From 990aeb016b08c2f426387b31cb0d16c7fe30e576 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 14:11:11 +0200 Subject: [PATCH 02/11] Set v0.6.0 in doxygen configuration --- docs/doxy.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/doxy.conf b/docs/doxy.conf index 6649f4143..d91dae080 100644 --- a/docs/doxy.conf +++ b/docs/doxy.conf @@ -54,7 +54,7 @@ PROJECT_NAME = "ALP/GraphBLAS" # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 0.5.0 +PROJECT_NUMBER = 0.6.0 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a From 5766f2f8db055dfe3926cacb9f42da7b141bef90 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 15:01:14 +0200 Subject: [PATCH 03/11] Minor code style fix --- include/graphblas/reference/compressed_storage.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/graphblas/reference/compressed_storage.hpp b/include/graphblas/reference/compressed_storage.hpp index b776b8c84..34d2e99f1 100644 --- a/include/graphblas/reference/compressed_storage.hpp +++ b/include/graphblas/reference/compressed_storage.hpp @@ -25,6 +25,7 @@ #include //std::memcpy + namespace grb { namespace internal { From 89959475f68fb46e70345277ebd10df39f6d1f3f Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 15:01:38 +0200 Subject: [PATCH 04/11] Add new suppression, and clarify rationale of an existing one --- include/graphblas/reference/blas1.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index 4c5a87a67..d0c9180ed 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -3017,7 +3017,9 @@ namespace grb { for( size_t i = 0; i < block_size; i++ ) { if( masked ) { if( mask[ i ] ) { - z_p[ offsets[ i ] ] = z_b[ i ]; + GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // if masked && mask[ i ], then + z_p[ offsets[ i ] ] = z_b[ i ]; // z_b[ i ] was set from x or y in + GRB_UTIL_RESTORE_WARNINGS // the above } } else { if( x_m[ i ] ) { @@ -8176,7 +8178,7 @@ namespace grb { if( mask[ k ] ) { GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // yy and xx cannot be used // uninitialised or mask - apply( zz[ k ], xx[ k ], yy[ k ], anyOp ); // would be false. also, zz + apply( zz[ k ], xx[ k ], yy[ k ], anyOp ); // would be false while zz GRB_UTIL_RESTORE_WARNINGS // init is just above } } From 3c1416776bcc0262afb644ff695332e1f50fa7eb Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 15:01:57 +0200 Subject: [PATCH 05/11] Move list of active suppressions into repository. --- docs/Suppressions.md | 57 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 docs/Suppressions.md diff --git a/docs/Suppressions.md b/docs/Suppressions.md new file mode 100644 index 000000000..78ec4d778 --- /dev/null +++ b/docs/Suppressions.md @@ -0,0 +1,57 @@ + +This file keeps track of all active compiler warning suppressions and updates +with every release. Listed are the code, which suppression is used, and the +rationale for why the suppression is OK to use and the compiler warning is safe +to ignore. + +1. `include/graphblas/reference/compressed_storage.hpp`, copyFrom: +``` +GRB_UTIL_IGNORE_CLASS_MEMACCESS // by the ALP spec, D can only be POD types. + // In this case raw memory copies are OK. +(void) std::memcpy( values + k, + other.values + k, + (loop_end - k) * sizeof( D ) +); +GRB_UTIL_RESTORE_WARNINGS +``` + +2. `include/graphblas/reference/blas1.hpp`, dot_generic: +``` +for( size_t k = 0; k < AnyOp::blocksize; ++k ) { + zz[ k ] = addMonoid.template getIdentity< typename AnyOp::D3 >(); +} +for( size_t k = 0; k < AnyOp::blocksize; ++k ) { + if( mask[ k ] ) { + GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // yy and xx cannot be used + // uninitialised or mask + apply( zz[ k ], xx[ k ], yy[ k ], anyOp ); // would be false while zz + GRB_UTIL_RESTORE_WARNINGS // init is just above + } +} +``` + +3. `include/graphblas/reference/blas1.hpp`, sparse_apply_generic: +``` +if( masked ) { + if( mask[ i ] ) { + GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // if masked && mask[ i ], then + z_p[ offsets[ i ] ] = z_b[ i ]; // z_b[ i ] was set from x or y in + GRB_UTIL_RESTORE_WARNINGS // the above + } +} else { + GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // the only way the below could write + // an uninitialised value is if the + // static_assert at the top of this + z_p[ offsets[ i ] ] = z_b[ i ]; // function had triggered. See also + GRB_UTIL_RESTORE_WARNINGS // internal issue #321. +} +``` + +4. `include/graphblas/base/internalops.hpp`, multiple sources: +- mul::apply, add::apply, add::foldl, equal::apply, not_equal::apply. +These are indirectly caused by the following calls: +- `include/graphblas/blas0.hpp`, apply; +- `include/graphblas/reference/blas1.hpp`, dot_generic, masked_apply_generic, + and sparse_apply_generic. +These are all OK to suppress since the reads are masked. + From 97ba1892c23a31052edd27e00a03316460667542 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 16:32:06 +0200 Subject: [PATCH 06/11] Changelog added for all changes since v0.5 --- changelog.md | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/changelog.md b/changelog.md index c99d57963..72aa3e4d6 100644 --- a/changelog.md +++ b/changelog.md @@ -1,4 +1,98 @@ +Version 0.6.0 +============= + +This is a summary of changes. For full details, see the publicly available Git +history prior to the v0.6 tag. + +Highlights and changes to the specification: + - Deprecated `grb::init` and `grb::finalize` in favour of grb::Launcher. + Existing code should migrate to using the Launcher as any later release may + remove the now-deprecated primtives. + - If you wish to rely on ALP/GraphBLAS for more standard sparse linear + algebra but if you cannot, or do not wish to, adapt your existing sources + to the C++ ALP/GraphBLAS API, then v0.6 onwards generates libraries that + implement a subset of the standard C/C++ SparseBLAS and SpBLAS interfaces. + After installation, these libraries are found in + - `/lib/sequential/libsparseblas.a` (sequential) and + - `/lib/sequential/libsparseblas_omp.a` (shared-memory + parallel). + The headers are found in + - `/include/transition/sparseblas.h` and + - `/include/transition/spblas.h`. + - Input iterators passed to `grb::buildMatrixUnique` that happen to be random + access will now lead to shared-memory parallel ingestion when using the + reference_omp or hybrid backends. + - `grb::Phase` is now only accepted for primitives with non-scalar + ALP/GraphBLAS output containers. + +Algorithms: + - Feature: the CG algorithm has been adapted to work with complex-valued + matrices making use of the standard `std::complex` type. A corresponding + smoke test is added. + - Bugfix: BiCGstab erroneously relied on `grb::utils::equals`, and could + (rarely) lead to false orthogonality detection and an unnecessary abort. + +Utilities: + - The parser that reads MatrixMarket files, `grb::utils::MatrixFileReader`, now + can parse complex values and load Hermitian matrices. + - What constitutes an ALP/GraphBLAS sparse matrix iterator has been formalised + with a novel type trait, `grb::utils::is_alp_matrix_iterator`. ALP/GraphBLAS + matrix output iterators now also adhere to these requirements. + - A `grb::utils::is_complex` type trait has been added, and is used by the CG + algorithm so as to not materialise unnecessary buffers and code paths. + - Bugfixes to the `grb::utils::equals` routine, as well as better + documentation. A unit test has been added for it. + +Testing, development, and documentation: + - Documentation has been adapted to include GitHub for reporting issues. + - Documentation of various ALP/GraphBLAS primitives and concepts have been + improved. + - Documentation detailing the compiler warning suppressions and their rationale + have been moved to the code repository at `docs/Suppressions.md`. + - Add basic CI tests (build, install, and smoke tests) for GitHub. + - More thorough testing of output matrix iterators, input iterators, and of + `grb::buildMatrixUnique`. + - The `dense_spmv.cpp` smoke test did not correctly verify output, and could + fail for SpMV multiplications that yield very small nonzero values. + - Improvements to various tests and scripts. + +Reference and reference_omp backends: + - Bugfix: matrix output iterators failed if all nonzeroes were on the last row + and no nonzeroes existed anywhere else. + - Bugfix: copying and immediately dereferencing a matrix output iterator led to + use of uninitialised values. + - Bugfix: `grb::foldl` with a dense descriptor would accept sparse inputs while + it should return `ILLEGAL`. This behaviour, as well as for other error codes, + are now also (unit) tested for, including with masks and inverted masks. + - Bugfix: `grb::set` was moved from `reference/blas1.hpp` to + `reference/io.hpp`, but the macros that guard parallelisation were not + properly updated. + - Bugfix: the OpenMP `schedule( static, chunk_size )` has a dynamic (run-time) + component that was not intended. + - Bugifx: some OpenMP `schedule( dynamic, chunk_size )` operate on regular + loops and should employ a static schedule instead. + +BSP1D backend: + - Bugfix: too thorough sanity checking disallowed building dense matrices. + - Bugfix: `grb::set` on vectors with non-fundamental value types would not + compile (due to code handling the use_index descriptor). + - Bugfix: `grb::clear` could leave the vector in an undefined state if it + immediately followed an operation that left said vector dense. + - Code improvement: PinnedVector constructors now throws exceptions on errors. + +All backends: + - Bugfix: an input-masked variant of `grb::foldr` was missing. These are now + also added to the unit test suite. + - Bugfix: matrix constructors that throw an exception could segfault on + destruction. + - Bugfix: use of PinnedVectors that pin sparse or empty vectors could segfault. + - Code improvements: noexcept additions, const-correctness, code style fixes, + removal of compiler warnings (on some compiler versions), dead code removal, + improved `_DEBUG` tracing, additional debug-mode sanity checks, and reduced + code duplication. + + Version 0.5.0 ============= @@ -123,6 +217,7 @@ Various other bugfixes, performance bug fixes, documentation improvements, default configuration updates, and code structure improvements -- see Gitee MRs !21, !22, !38, !39, !40, and !42 for details. + Version 0.4.1 ============= From c49de7e394317174bc78ef00edfe949dcb307c86 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 16:41:45 +0200 Subject: [PATCH 07/11] Add rationale of suppression to code and doc --- docs/Suppressions.md | 25 +++++++++++++++++++++++++ include/graphblas/reference/blas1.hpp | 12 ++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/docs/Suppressions.md b/docs/Suppressions.md index 78ec4d778..1e968ef18 100644 --- a/docs/Suppressions.md +++ b/docs/Suppressions.md @@ -55,3 +55,28 @@ These are indirectly caused by the following calls: and sparse_apply_generic. These are all OK to suppress since the reads are masked. +5. `include/graphblas/reference/blas1.hpp`, fold_from_vector_to_scalar_generic: +``` +GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // the below code ensures to set local +IOType local; // whenever our local block is +GRB_UTIL_RESTORE_WARNINGS // non-empty +if( end > 0 ) { + if( i < end ) { + local = static_cast< IOType >( internal::getRaw( to_fold )[ i ] ); + } else { + local = static_cast< IOType >( internal::getRaw( to_fold )[ 0 ] ); + } +} +``` +and +``` +if( root == s ) { + // then I should be non-empty + assert( !empty ); + // set global value to locally computed value + GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // one is only root if the local + global = local; // chunk is non-empty, in which case + GRB_UTIL_RESTORE_WARNINGS // local will be initialised (above) + } +``` + diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index d0c9180ed..b4e6d4a99 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -244,9 +244,9 @@ namespace grb { } #endif - GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED - IOType local; - GRB_UTIL_RESTORE_WARNINGS + GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // the below code ensures to set local + IOType local; // whenever our local block is + GRB_UTIL_RESTORE_WARNINGS // non-empty if( end > 0 ) { if( i < end ) { local = static_cast< IOType >( internal::getRaw( to_fold )[ i ] ); @@ -316,9 +316,9 @@ namespace grb { // then I should be non-empty assert( !empty ); // set global value to locally computed value - GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED - global = local; - GRB_UTIL_RESTORE_WARNINGS + GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // one is only root if the local + global = local; // chunk is non-empty, in which case + GRB_UTIL_RESTORE_WARNINGS // local will be initialised (above) } } #pragma omp barrier From 8ef4e84b3c8d4761f5ba1c4d0a5696ebcdd588ec Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 17:05:36 +0200 Subject: [PATCH 08/11] Minor code style fixes --- include/graphblas/internalops.hpp | 3 ++- include/graphblas/utils/suppressions.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/include/graphblas/internalops.hpp b/include/graphblas/internalops.hpp index 9b02d4aa2..d07908e81 100644 --- a/include/graphblas/internalops.hpp +++ b/include/graphblas/internalops.hpp @@ -29,7 +29,8 @@ #include "base/internalops.hpp" #ifdef _GRB_WITH_BANSHEE -#include + #include #endif #endif + diff --git a/include/graphblas/utils/suppressions.h b/include/graphblas/utils/suppressions.h index 1e364abde..e40946c55 100644 --- a/include/graphblas/utils/suppressions.h +++ b/include/graphblas/utils/suppressions.h @@ -50,5 +50,5 @@ #define GRB_UTIL_RESTORE_WARNINGS #endif -#endif // end ``_H_GRB_REFERENCE_BLAS2'' +#endif // end ``_H_GRB_UTILS_SUPRESSIONS'' From 48bb36ab5a725ea7057b62f9cfda4e0c6bc22f30 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 17:10:17 +0200 Subject: [PATCH 09/11] Formatting fixes --- docs/Suppressions.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/Suppressions.md b/docs/Suppressions.md index 1e968ef18..4f7db5921 100644 --- a/docs/Suppressions.md +++ b/docs/Suppressions.md @@ -49,10 +49,12 @@ if( masked ) { 4. `include/graphblas/base/internalops.hpp`, multiple sources: - mul::apply, add::apply, add::foldl, equal::apply, not_equal::apply. + These are indirectly caused by the following calls: - `include/graphblas/blas0.hpp`, apply; - `include/graphblas/reference/blas1.hpp`, dot_generic, masked_apply_generic, and sparse_apply_generic. + These are all OK to suppress since the reads are masked. 5. `include/graphblas/reference/blas1.hpp`, fold_from_vector_to_scalar_generic: From e33233849cc768cab5825289565af5c45a80d9f7 Mon Sep 17 00:00:00 2001 From: "Albert-Jan N. Yzelman" Date: Wed, 10 Aug 2022 17:20:51 +0200 Subject: [PATCH 10/11] Release candidates should be subject to the full testing pipeline --- .gitlab-ci.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d843432c4..c6a81c040 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -23,6 +23,7 @@ stages: # only: # - master # - develop +# - /^v.*-rc.*$/ # artifacts: # paths: # - build/ @@ -44,6 +45,7 @@ stages: # only: # - master # - develop +# - /^v.*-rc.*$/ #test_centos_8_unit: @@ -59,6 +61,7 @@ stages: # only: # - master # - develop +# - /^v.*-rc.*$/ #test_centos_8_smoke: @@ -74,6 +77,7 @@ stages: # only: # - master # - develop +# - /^v.*-rc.*$/ #test_centos_8_performance: @@ -89,6 +93,7 @@ stages: # only: # - master # - develop +# - /^v.*-rc.*$/ #test_centos_8_installation: @@ -103,6 +108,7 @@ stages: # only: # - master # - develop +# - /^v.*-rc.*$/ # Main testing on Ubuntu, all branches @@ -148,6 +154,7 @@ build_debugrelease_tests: only: - master - develop + - /^v.*-rc.*$/ # linting is currently disabled since clang-format is not mature enough @@ -266,6 +273,7 @@ tests_performance: only: - master - develop + - /^v.*-rc.*$/ tests_unit_debug: @@ -281,4 +289,5 @@ tests_unit_debug: only: - master - develop + - /^v.*-rc.*$/ From 12322b73421e79700b6d10225b6b53b3ecd59cd4 Mon Sep 17 00:00:00 2001 From: Albert-Jan Yzelman Date: Wed, 10 Aug 2022 19:09:54 +0200 Subject: [PATCH 11/11] Compilation on ARM revealed an additionally necessary suppression (GCC 9.4.0), shorter SIMD width) --- docs/Suppressions.md | 9 +++++++++ include/graphblas/reference/blas1.hpp | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/Suppressions.md b/docs/Suppressions.md index 4f7db5921..1915147b5 100644 --- a/docs/Suppressions.md +++ b/docs/Suppressions.md @@ -82,3 +82,12 @@ if( root == s ) { } ``` +6. `include/graphblas/reference/blas1.hpp`, masked_apply_generic: +``` +if( mask_b[ t ] ) { + // ... + GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // z_b is computed from x_b and + *( z_p + indices[ t ] ) = z_b[ t ]; // y_b, which are both initialised + GRB_UTIL_RESTORE_WARNINGS // if mask_b is true +``` + diff --git a/include/graphblas/reference/blas1.hpp b/include/graphblas/reference/blas1.hpp index b4e6d4a99..df14f6545 100644 --- a/include/graphblas/reference/blas1.hpp +++ b/include/graphblas/reference/blas1.hpp @@ -3433,7 +3433,9 @@ namespace grb { } #endif } - *( z_p + indices[ t ] ) = z_b[ t ]; + GRB_UTIL_IGNORE_MAYBE_UNINITIALIZED // z_b is computed from x_b and + *( z_p + indices[ t ] ) = z_b[ t ]; // y_b, which are both initialised + GRB_UTIL_RESTORE_WARNINGS // if mask_b is true } } #ifndef _H_GRB_REFERENCE_OMP_BLAS1