From 0ad370a0efaf9df60c5e3fa7c46ddebdfeca57c2 Mon Sep 17 00:00:00 2001 From: asadchev Date: Tue, 9 Mar 2021 16:29:24 -0500 Subject: [PATCH 1/3] Refactor TA_ASSERT --- CMakeLists.txt | 33 +++------ src/TiledArray/config.h.in | 9 ++- src/TiledArray/dist_array.h | 2 +- src/TiledArray/error.h | 124 +++++++++++---------------------- src/TiledArray/tiledarray.cpp | 9 +++ tests/ta_test.cpp | 5 +- tests/tot_dist_array_part2.cpp | 8 --- 7 files changed, 70 insertions(+), 120 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 42cccfa424..56cf6d8ea4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -124,7 +124,7 @@ if((ENABLE_GPERFTOOLS OR ENABLE_TCMALLOC_MINIMAL) AND CMAKE_SYSTEM_NAME MATCHES set(ENABLE_LIBUNWIND ON) add_feature_info(Libunwind ENABLE_LIBUNWIND "Libunwind provides stack unwinding") endif() -option(TA_BUILD_UNITTEST "Causes building TiledArray unit tests" OFF) +option(TA_BUILD_UNITTEST "Causes building TiledArray unit tests" ON) option(TA_EXPERT "TiledArray Expert mode: disables automatically downloading or building dependencies" OFF) option(TA_SIGNED_1INDEX_TYPE "Enables the use of signed 1-index coordinate type (OFF in 1.0.0-alpha.2 and older)" ON) @@ -255,28 +255,17 @@ include(CheckTypeSize) check_type_size("long double" TILEDARRAY_HAS_LONG_DOUBLE LANGUAGE CXX) check_type_size("long long" TILEDARRAY_HAS_LONG_LONG LANGUAGE CXX) -########################## -# convert string values of TA_ERROR to numerical values expected by TA_DEFAULT_ERROR -########################## -set (TA_DEFAULT_ERROR 3) # default is to abort so that it works with or without NDEBUG - # assert when CMAKE_BUILD_TYPE is Debug or RelWithDebInfo -string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE) -if (CMAKE_BUILD_TYPE AND uppercase_CMAKE_BUILD_TYPE MATCHES "^(DEBUG|RELWITHDEBINFO)$") - set (TA_DEFAULT_ERROR 2) -endif() +# TA_ASSERT +set (TA_ASSERT_POLICY TA_ASSERT_THROW CACHE STRING "") +set_property( + CACHE TA_ASSERT_POLICY PROPERTY + STRINGS TA_ASSERT_THROW TA_ASSERT_ABORT TA_ASSERT_IGNORE) + # if building unit tests default to throw to be able to test TA_ASSERT statements -if (TA_BUILD_UNITTEST) - set (TA_DEFAULT_ERROR 1) -endif(TA_BUILD_UNITTEST) - -if (TA_ERROR STREQUAL none) - set (TA_DEFAULT_ERROR 0) -elseif (TA_ERROR STREQUAL throw) - set (TA_DEFAULT_ERROR 1) -elseif (TA_ERROR STREQUAL assert) - set (TA_DEFAULT_ERROR 2) -elseif (TA_ERROR STREQUAL abort) - set (TA_DEFAULT_ERROR 3) +if (NOT (TA_ASSERT_POLICY STREQUAL TA_ASSERT_THROW)) + if (TA_BUILD_UNITTEST) + message(FATAL_ERROR "TA_ASSERT_POLICY=${TA_ASSERT_POLICY} requires TA_BUILD_UNITTEST=OFF") + endif(TA_BUILD_UNITTEST) endif() ########################## diff --git a/src/TiledArray/config.h.in b/src/TiledArray/config.h.in index f26da58c13..8e66bf7159 100644 --- a/src/TiledArray/config.h.in +++ b/src/TiledArray/config.h.in @@ -53,8 +53,11 @@ #endif -/* Defines the default error checking behavior. none = 0, throw = 1, assert = 2, abort = 3 */ -#define TA_DEFAULT_ERROR @TA_DEFAULT_ERROR@ +/* Defines the default error checking behavior */ +#define TA_ASSERT_THROW 2 +#define TA_ASSERT_ABORT 3 +#define TA_ASSERT_IGNORE 4 +#define TA_ASSERT_POLICY @TA_ASSERT_POLICY@ /* define if compiler supports long double, the value is sizeof(long double) */ #cmakedefine TILEDARRAY_HAS_LONG_DOUBLE 1 @@ -69,7 +72,7 @@ #cmakedefine TILEDARRAY_CACHELINE_SIZE @TILEDARRAY_CACHELINE_SIZE@ /* Define if TA has enabled ScaLAPACK Bindings */ -#cmakedefine TILEDARRAY_HAS_SCALAPACK 1 +#cmakedefine TILEDARRAY_HAS_SCALAPACK 1 /* Define if TiledArray configured with CUDA support */ #cmakedefine TILEDARRAY_HAS_CUDA @TILEDARRAY_HAS_CUDA@ diff --git a/src/TiledArray/dist_array.h b/src/TiledArray/dist_array.h index 4ed557613c..0378c01f43 100644 --- a/src/TiledArray/dist_array.h +++ b/src/TiledArray/dist_array.h @@ -1487,7 +1487,7 @@ class DistArray : public madness::archive::ParallelSerializableObject { /// @warning this tests contents of @p vars using #TA_ASSERT() only if /// preprocessor macro @c NDEBUG is not defined void check_str_index(const std::string& vars) const { -#ifndef NDEBUG +#if (TA_ASSERT_POLICY != TA_ASSERT_IGNORE) // Only check indices if the PIMPL is initialized (okay to not initialize // the RHS of an equation) if (!is_initialized()) return; diff --git a/src/TiledArray/error.h b/src/TiledArray/error.h index 147e9fe7f4..87a2ca1052 100644 --- a/src/TiledArray/error.h +++ b/src/TiledArray/error.h @@ -22,43 +22,57 @@ #include -// Check for default error checking method, which is determined by -// TA_DEFAULT_ERROR, which is defined in TiledArray/config.h. -#ifdef TA_DEFAULT_ERROR -#if !defined(TA_EXCEPTION_ERROR) && !defined(TA_ASSERT_ERROR) && \ - !defined(TA_NO_ERROR) && !defined(TA_ABORT_ERROR) -#if TA_DEFAULT_ERROR == 0 -#define TA_NO_ERROR -#elif TA_DEFAULT_ERROR == 1 -#define TA_EXCEPTION_ERROR -#elif TA_DEFAULT_ERROR == 2 -#define TA_ASSERT_ERROR -#elif TA_DEFAULT_ERROR == 3 -#define TA_ABORT_ERROR -#endif // TA_DEFAULT_ERROR == ? -#endif // !defined(TA_EXCEPTION_ERROR) && !defined(TA_ASSERT_ERROR) && - // !defined(TA_NO_ERROR) && !defined(TA_ABORT_ERROR) -#endif // TA_DEFAULT_ERROR - -#include +#ifndef TA_ASSERT_POLICY +#define TA_ASSERT_POLICY TA_ASSERT_THROW +#endif + +#define TA_STRINGIZE_IMPL(s) #s +#define TA_STRINGIZE(s) TA_STRINGIZE_IMPL(s) + +#define TA_ASSERT_MESSAGE(EXPR, ...) \ + __FILE__ ":" TA_STRINGIZE(__LINE__) ": " \ + "TA_ASSERT failed: " TA_STRINGIZE(EXPR) + +#if TA_ASSERT_POLICY == TA_ASSERT_IGNORE +#define TA_ASSERT(...) do { } while(0) +#else +#define TA_ASSERT(EXPR, ...) \ + do { \ + if (!(EXPR)) \ + TiledArray::assert_failed(TA_ASSERT_MESSAGE(EXPR, __VA_ARGS__)); \ + } while(0) + +#endif + +#include +#include + namespace TiledArray { -class Exception : public std::exception { - public: - Exception(const char* m) : message_(m) {} +void ta_abort(); - virtual const char* what() const noexcept { return message_; } +void ta_abort(const std::string &m); - private: - const char* message_; +class Exception : public std::runtime_error { + using std::runtime_error::runtime_error; }; // class Exception /// Place a break point on this function to stop before TiledArray exceptions /// are thrown. inline void exception_break() {} -} // namespace TiledArray -#define TA_STRINGIZE(s) #s +inline void assert_failed(const std::string &m) { +#if TA_ASSERT_POLICY == TA_ASSERT_THROW + TiledArray::exception_break(); + throw TiledArray::Exception(m); +#elif TA_ASSERT_POLICY == TA_ASSERT_ABORT + TiledArray::ta_abort(m); +#elif TA_ASSERT_POLICY != TA_ASSERT_IGNORE +#error Invalid TA_ASSERT_POLICY parameter +#endif +} + +} // namespace TiledArray #define TA_EXCEPTION_MESSAGE(file, line, mess) \ "TiledArray: exception at " file "(" TA_STRINGIZE(line) "): " mess @@ -72,62 +86,6 @@ inline void exception_break() {} throw TiledArray::Exception(TA_EXCEPTION_MESSAGE(__FILE__, __LINE__, m)); \ } while (0) -#ifdef TA_EXCEPTION_ERROR -// This section defines the behavior for TiledArray assertion error checking -// which will throw exceptions. -#ifdef TA_ASSERT_ERROR -#undef TA_ASSERT_ERROR -#endif -#ifdef TA_ABORT_ERROR -#undef TA_ABORT_ERROR -#endif - -/// TiledArray assertion is configured to throw TiledArray::Exception -/// if \p a is false -/// \param a an expression convertible to bool -#define TA_ASSERT(a) \ - do { \ - if (!(a)) TA_EXCEPTION("assertion failure"); \ - } while (0) - -#elif defined(TA_ASSERT_ERROR) -// This sections defines behavior for TiledArray assertion error checking which -// uses assertions. -#include - -/// TiledArray assertion is configured to call \c assert() -/// if \p a is false -/// \param a an expression convertible to bool -#define TA_ASSERT(a) assert(a) - -#elif defined(TA_ABORT_ERROR) -// This sections defines behavior for TiledArray assertion error checking which -// calls std::abort -#include - -/// TiledArray assertion is configured to call std::abort if \p a is false -/// \param a an expression convertible to bool -#define TA_ASSERT(a) \ - do { \ - if (!(a)) std::abort(); \ - } while (0) - -#else -// This section defines behavior for TiledArray assertion error checking which -// does no error checking. -// WARNING: TiledArray will perform no error checking. -// this avoids unused variable warnings, see -// http://cnicholson.net/2009/02/stupid-c-tricks-adventures-in-assert/ - -/// TiledArray assertion is configured to do nothing -/// \param a an expression convertible to bool -#define TA_ASSERT(a) \ - do { \ - (void)sizeof(a); \ - } while (0) - -#endif // TA_EXCEPTION_ERROR - #ifdef TILEDARRAY_NO_USER_ERROR_MESSAGES #define TA_USER_ERROR_MESSAGE(m) #else diff --git a/src/TiledArray/tiledarray.cpp b/src/TiledArray/tiledarray.cpp index 44bcabd788..4f6aa96176 100644 --- a/src/TiledArray/tiledarray.cpp +++ b/src/TiledArray/tiledarray.cpp @@ -132,3 +132,12 @@ void TiledArray::finalize() { initialized_accessor() = false; finalized_accessor() = true; } + +void TiledArray::ta_abort() { + std::abort(); +} + +void TiledArray::ta_abort(const std::string &m) { + std::cerr << m << std::endl; + ta_abort(); +} diff --git a/tests/ta_test.cpp b/tests/ta_test.cpp index c342e7523a..8e2bc45705 100644 --- a/tests/ta_test.cpp +++ b/tests/ta_test.cpp @@ -24,9 +24,8 @@ #include "unit_test_config.h" #include -#ifndef TA_EXCEPTION_ERROR -#error \ - "TiledArray unit tests can only be built if CMake variable TA_ERROR is set of \"throw\"" +#if (TA_ASSERT_POLICY != TA_ASSERT_THROW) +#error "TiledArray unit tests require TA_ASSERT_POLICY=TA_ASSERT_THROW" #endif GlobalFixture::GlobalFixture() { diff --git a/tests/tot_dist_array_part2.cpp b/tests/tot_dist_array_part2.cpp index 30c3634a40..ed3b2b71af 100644 --- a/tests/tot_dist_array_part2.cpp +++ b/tests/tot_dist_array_part2.cpp @@ -348,17 +348,13 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(call_operator, TestParam, test_params) { std::string outer_idx = (outer_rank == 1 ? "i" : "i,j"); std::string inner_idx = (inner_rank == 1 ? "k" : "k,l"); - // call operators validate inputs only if NDEBUG not defined -#ifndef NDEBUG if (m_world.nproc() == 1) { using except_t = TiledArray::Exception; // Throws if no semicolon BOOST_CHECK_THROW(t(outer_idx), except_t); - // Throws if wrong outer rank BOOST_CHECK_THROW(t("i,j,k,l,m;" + inner_idx), except_t); } -#endif auto vars = outer_idx + ";" + inner_idx; auto expr = t(vars); @@ -375,17 +371,13 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(const_call_operator, TestParam, test_params) { std::string outer_idx = (outer_rank == 1 ? "i" : "i,j"); std::string inner_idx = (inner_rank == 1 ? "k" : "k,l"); - // call operators validate inputs only if NDEBUG not defined -#ifndef NDEBUG if (m_world.nproc() == 1) { using except_t = TiledArray::Exception; // Throws if no semicolon BOOST_CHECK_THROW(t(outer_idx), except_t); - // Throws if wrong outer rank BOOST_CHECK_THROW(t("i,j,k,l,m;" + inner_idx), except_t); } -#endif auto vars = outer_idx + ";" + inner_idx; auto expr = t(vars); From 77a0dae9b63106490facf9ed3ba0c6137df4433e Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sun, 14 Mar 2021 15:56:53 -0400 Subject: [PATCH 2/3] dox --- INSTALL.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index 42b473da4e..e13950f20d 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -135,7 +135,6 @@ $ cmake -D CMAKE_INSTALL_PREFIX=/path/to/install/tiledarray \ ``` $ cmake -D CMAKE_INSTALL_PREFIX=/path/to/install/tiledarray \ -D CMAKE_BUILD_TYPE=Debug \ - -D TA_BUILD_UNITTEST=ON \ -D BOOST_ROOT=/path/to/boost \ -D CMAKE_PREFIX_PATH=/path/to/dependency;/path/to/another/dependency \ $TILEDARRAY_SOURCE_DIR @@ -349,7 +348,8 @@ support may be added. ## Expert configure options: * `TA_EXPERT` -- Set to `ON` to disable automatic installation of prerequisites. Useful for experts, hence the name. [Default=OFF]. -* `TA_ERROR` -- Set to `none` to disable `TA_ASSERT` assertions, `throw` to cause `TA_ASSERT` assertions to throw, `abort` to cause `TA_ASSERT` assertions to abort, or `assert` to cause `TA_ASSERT` assertions to use C++ assert. The default is `throw` if `TA_BUILD_UNITTEST` is set, else is `assert` if `CMAKE_BUILD_TYPE` is `Debug` or `RelWithDebInfo`, else is `abort`. +* `TA_ASSERT_POLICY` -- Set to `TA_ASSERT_IGNORE` to disable `TA_ASSERT` assertions, `TA_ASSERT_THROW` to cause `TA_ASSERT` assertions to throw, `TA_ASSERT_ABORT` to cause `TA_ASSERT` assertions to abort. The default is `TA_ASSERT_THROW`. +* `TA_BUILD_UNITTEST` -- Set of `OFF` to disable building unit tests. The default is `ON`. * `TA_TRACE_TASKS` -- Set to `ON` to enable tracing of MADNESS tasks using custom task tracer. Note that standard profilers/tracers are generally useless (except in the trivial cases) with MADWorld-based programs since the submission context of tasks is not captured by standard tracing tools; this makes it impossible in a nontrivial program to attribute tasks to source code. WARNING: task tracing his will greatly increase the memory requirements. [Default=OFF]. * `TA_ENABLE_RANGEV3` -- Set to `ON` to find or fetch the Range-V3 library and enable additional tests of TA components with constructs anticipated to be supported in the future. [Default=OFF]. * `TA_SIGNED_1INDEX_TYPE` -- Set to `OFF` to use unsigned 1-index coordinate type (default for TiledArray 1.0.0-alpha.2 and older). The default is `ON`, which enables the use of negative indices in coordinates. From 9394446529920d768a40f58fb2db69acbc844cad Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sun, 14 Mar 2021 15:57:13 -0400 Subject: [PATCH 3/3] removed unnecessary TA_BUILD_UNITTEST=ON --- .github/workflows/ci.yml | 1 - .gitlab-ci.yml | 1 - bin/build-linux.sh | 2 -- bin/docker-build.sh | 2 +- 4 files changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 999c00f459..ed5b9001a8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,6 @@ jobs: CXX : ${{ matrix.cxx }} BUILD_CONFIG : > -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} - -DTA_BUILD_UNITTEST=ON -DMPIEXEC_PREFLAGS='--bind-to;none;--allow-run-as-root' steps: diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ee54cf681a..ed72df3e72 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -9,7 +9,6 @@ variables: MAD_NUM_THREADS : 2 TA_TARGETS : "tiledarray examples ta_test check" TA_CONFIG : > - TA_BUILD_UNITTEST=TRUE CMAKE_BUILD_TYPE=${BUILD_TYPE} ${TA_PYTHON} ${ENABLE_CUDA} diff --git a/bin/build-linux.sh b/bin/build-linux.sh index bf5d1391b6..bf27d220fe 100755 --- a/bin/build-linux.sh +++ b/bin/build-linux.sh @@ -96,7 +96,6 @@ if [ "$BUILD_TYPE" = "Debug" ]; then -DCMAKE_PREFIX_PATH="${INSTALL_PREFIX}/madness;${INSTALL_PREFIX}/eigen3;${INSTALL_PREFIX}/boost" \ -DPYTHON_EXECUTABLE="${PYTHON_EXECUTABLE}" \ -DTA_PYTHON="${TA_PYTHON}" \ - -DTA_BUILD_UNITTEST=ON \ -DENABLE_SCALAPACK=ON else @@ -126,7 +125,6 @@ else -DCMAKE_PREFIX_PATH="${INSTALL_PREFIX}/eigen3;${INSTALL_PREFIX}/boost" \ -DPYTHON_EXECUTABLE="${PYTHON_EXECUTABLE}" \ -DTA_PYTHON="${TA_PYTHON}" \ - -DTA_BUILD_UNITTEST=ON \ -DENABLE_SCALAPACK=ON fi diff --git a/bin/docker-build.sh b/bin/docker-build.sh index 5deb1fa9da..a0d052b497 100755 --- a/bin/docker-build.sh +++ b/bin/docker-build.sh @@ -35,7 +35,7 @@ RUN apt-get update && apt-get install -y python3 ninja-build liblapacke-dev libl RUN CMAKE_URL="https://cmake.org/files/v${CMAKE_VERSION%.[0-9]}/cmake-${CMAKE_VERSION}-Linux-x86_64.tar.gz" && wget --no-check-certificate -O - \$CMAKE_URL | tar --strip-components=1 -xz -C /usr/local ENV CMAKE=/usr/local/bin/cmake # 3. download and build TiledArray -RUN cd /usr/local/src && git clone --depth=1 https://github.com/ValeevGroup/tiledarray.git && cd /usr/local/src/tiledarray && mkdir build && cd build && \$CMAKE .. -G Ninja -DCMAKE_CXX_COMPILER=clang++-8 -DCMAKE_C_COMPILER=clang-8 -DTA_BUILD_UNITTEST=ON -DCMAKE_INSTALL_PREFIX=/usr/local -DCMAKE_BUILD_TYPE=RelWithDebInfo && \$CMAKE --build . --target tiledarray && \$CMAKE --build . --target check && $CMAKE --build . --target examples && \$CMAKE --build . --target install +RUN cd /usr/local/src && git clone --depth=1 https://github.com/ValeevGroup/tiledarray.git && cd /usr/local/src/tiledarray && mkdir build && cd build && \$CMAKE .. -G Ninja -DCMAKE_CXX_COMPILER=clang++-8 -DCMAKE_C_COMPILER=clang-8 -DCMAKE_INSTALL_PREFIX=/usr/local -DCMAKE_BUILD_TYPE=RelWithDebInfo && \$CMAKE --build . --target tiledarray && \$CMAKE --build . --target check && $CMAKE --build . --target examples && \$CMAKE --build . --target install # Clean up APT when done. RUN apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*