From b9697ec8a70323f314a29223aa60cf5bb8de5684 Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sat, 19 Dec 2020 13:51:02 -0500 Subject: [PATCH 01/12] put small Range objects on stack --- src/TiledArray/block_range.h | 4 +- src/TiledArray/range.h | 88 +++++++++++++++++------------------- 2 files changed, 43 insertions(+), 49 deletions(-) diff --git a/src/TiledArray/block_range.h b/src/TiledArray/block_range.h index 661e38a205..cb2ab25336 100644 --- a/src/TiledArray/block_range.h +++ b/src/TiledArray/block_range.h @@ -50,7 +50,7 @@ class BlockRange : public Range { TA_ASSERT(range.rank()); // Initialize the block range data members - data_ = new index1_type[range.rank() << 2]; + init_datavec(range.rank()); offset_ = range.offset(); volume_ = 1ul; rank_ = range.rank(); @@ -102,7 +102,7 @@ class BlockRange : public Range { TA_ASSERT(range.rank()); // Initialize the block range data members - data_ = new index1_type[range.rank() << 2]; + init_datavec(range.rank()); offset_ = range.offset(); volume_ = 1ul; rank_ = range.rank(); diff --git a/src/TiledArray/range.h b/src/TiledArray/range.h index 5896fd3ff7..6b882583f5 100644 --- a/src/TiledArray/range.h +++ b/src/TiledArray/range.h @@ -60,6 +60,7 @@ class Range { static_assert(detail::is_range_v); // index is a Range protected: + container::svector datavec_; index1_type* data_ = nullptr; ///< An array that holds the dimension information of the ///< range. The layout of the array is: @@ -73,6 +74,14 @@ class Range { ordinal_type volume_ = 0ul; ///< Total number of elements unsigned int rank_ = 0u; ///< The rank (or number of dimensions) in the range + void init_datavec(unsigned int rank) { + if (rank > 0) { + datavec_.resize(rank << 2); + data_ = datavec_.data(); + } else + data_ = nullptr; + } + private: /// Initialize range data from sequences of lower and upper bounds @@ -343,7 +352,7 @@ class Range { TA_ASSERT(n == size(upper_bound)); if (n) { // Initialize array memory - data_ = new index1_type[n << 2]; + init_datavec(n); rank_ = n; init_range_data(lower_bound, upper_bound); } @@ -376,10 +385,9 @@ class Range { using std::size; const auto n = size(lower_bound); TA_ASSERT(n == size(upper_bound)); + init_datavec(n); + rank_ = n; if (n) { - // Initialize array memory - data_ = new index1_type[n << 2]; - rank_ = n; init_range_data(lower_bound, upper_bound); } } @@ -402,7 +410,7 @@ class Range { const auto n = size(extent); if (n) { // Initialize array memory - data_ = new index1_type[n << 2]; + init_datavec(n); rank_ = n; init_range_data(extent); } @@ -425,7 +433,7 @@ class Range { const auto n = size(extent); if (n) { // Initialize array memory - data_ = new index1_type[n << 2]; + init_datavec(n); rank_ = n; init_range_data(extent); } @@ -467,7 +475,7 @@ class Range { const auto n = std::size(bounds); if (n) { // Initialize array memory - data_ = new index1_type[n << 2]; + init_datavec(n); rank_ = n; init_range_data(bounds); } @@ -498,7 +506,7 @@ class Range { const auto n = size(bounds); if (n) { // Initialize array memory - data_ = new index1_type[n << 2]; + init_datavec(n); rank_ = n; init_range_data(bounds); } @@ -531,7 +539,7 @@ class Range { } #endif // Initialize array memory - data_ = new index1_type[n << 2]; + init_datavec(n); rank_ = n; init_range_data(bounds); } @@ -574,11 +582,11 @@ class Range { /// \param other The range to be copied Range(const Range_& other) { if (other.rank_ > 0ul) { - data_ = new index1_type[other.rank_ << 2]; + datavec_ = other.datavec_; + data_ = datavec_.data(); offset_ = other.offset_; volume_ = other.volume_; rank_ = other.rank_; - memcpy(data_, other.data_, (sizeof(index1_type) << 2) * other.rank_); } } @@ -586,7 +594,8 @@ class Range { /// \param other The range to be copied Range(Range_&& other) - : data_(other.data_), + : datavec_(std::move(other.datavec_)), + data_(datavec_.data()), offset_(other.offset_), volume_(other.volume_), rank_(other.rank_) { @@ -604,14 +613,15 @@ class Range { TA_ASSERT(perm.size() == other.rank_); if (other.rank_ > 0ul) { - data_ = new index1_type[other.rank_ << 2]; rank_ = other.rank_; if (perm) { + init_datavec(other.rank_); init_range_data(perm, other.lobound_data(), other.upbound_data()); } else { // Simple copy will do - memcpy(data_, other.data_, (sizeof(index1_type) << 2) * rank_); + datavec_ = other.datavec_; + data_ = datavec_.data(); offset_ = other.offset_; volume_ = other.volume_; } @@ -619,19 +629,16 @@ class Range { } /// Destructor - ~Range() { delete[] data_; } + ~Range() { data_ = nullptr; } /// Copy assignment operator /// \param other The range to be copied /// \return A reference to this object Range_& operator=(const Range_& other) { - if (rank_ != other.rank_) { - delete[] data_; - data_ = (other.rank_ > 0ul ? new index1_type[other.rank_ << 2] : nullptr); - rank_ = other.rank_; - } - memcpy(data_, other.data_, (sizeof(index1_type) << 2) * rank_); + datavec_ = other.datavec_; + data_ = datavec_.data(); + rank_ = other.rank_; offset_ = other.offset_; volume_ = other.volume_; @@ -644,7 +651,8 @@ class Range { /// \return A reference to this object /// \throw nothing Range_& operator=(Range_&& other) { - data_ = other.data_; + datavec_ = std::move(other.datavec_); + data_ = datavec_.data(); offset_ = other.offset_; volume_ = other.volume_; rank_ = other.rank_; @@ -903,8 +911,7 @@ class Range { // Reallocate memory for range arrays if (rank_ != n) { - delete[] data_; - data_ = (n > 0ul ? new index1_type[n << 2] : nullptr); + init_datavec(n); rank_ = n; } if (n > 0ul) @@ -1112,32 +1119,22 @@ class Range { typename std::enable_if::value>::type* = nullptr> void serialize(const Archive& ar) { - // Get rank - unsigned int rank = 0ul; - ar& rank; - - // Reallocate the array - const unsigned int four_x_rank = rank << 2; - if (rank_ != rank) { - delete[] data_; - data_ = (rank > 0u ? new index1_type[four_x_rank] : nullptr); - rank_ = rank; - } - - // Get range data - ar& madness::archive::wrap(data_, four_x_rank) & offset_& volume_; + ar& rank_& datavec_& offset_& volume_; + data_ = datavec_.data(); } template ::value>::type* = nullptr> void serialize(const Archive& ar) const { - ar& rank_& madness::archive::wrap(data_, rank_ << 2) & offset_& volume_; + ar& rank_& datavec_& offset_& volume_; } void swap(Range_& other) { // Get temp data - std::swap(data_, other.data_); + std::swap(datavec_, other.datavec_); + data_ = datavec_.data(); + other.data_ = other.datavec_.data(); std::swap(offset_, other.offset_); std::swap(volume_, other.volume_); std::swap(rank_, other.rank_); @@ -1233,14 +1230,11 @@ inline Range& Range::operator*=(const Permutation& perm) { TA_ASSERT(perm.size() == rank_); if (rank_ > 1ul) { // Copy the lower and upper bound data into a temporary array - auto* MADNESS_RESTRICT const temp_lower = new index1_type[rank_ << 1]; - const auto* MADNESS_RESTRICT const temp_upper = temp_lower + rank_; - std::memcpy(temp_lower, data_, (sizeof(index1_type) << 1) * rank_); - - init_range_data(perm, temp_lower, temp_upper); + container::svector temp_lower(rank_ << 1); + const auto* MADNESS_RESTRICT const temp_upper = temp_lower.data() + rank_; + std::memcpy(temp_lower.data(), data_, (sizeof(index1_type) << 1) * rank_); - // Cleanup old memory. - delete[] temp_lower; + init_range_data(perm, temp_lower.data(), temp_upper); } return *this; } From 8bcc54c1bd5cc26f59272472c47e0c675275db1f Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sun, 20 Dec 2020 09:17:26 -0500 Subject: [PATCH 02/12] [WIP] converting Range to use container::svector --- src/TiledArray/range.h | 54 ++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/src/TiledArray/range.h b/src/TiledArray/range.h index 6b882583f5..a00d60412a 100644 --- a/src/TiledArray/range.h +++ b/src/TiledArray/range.h @@ -61,7 +61,6 @@ class Range { protected: container::svector datavec_; - index1_type* data_ = nullptr; ///< An array that holds the dimension information of the ///< range. The layout of the array is: ///< \code @@ -74,13 +73,9 @@ class Range { ordinal_type volume_ = 0ul; ///< Total number of elements unsigned int rank_ = 0u; ///< The rank (or number of dimensions) in the range - void init_datavec(unsigned int rank) { - if (rank > 0) { - datavec_.resize(rank << 2); - data_ = datavec_.data(); - } else - data_ = nullptr; - } + void init_datavec(unsigned int rank) { datavec_.resize(rank << 2); } + const index1_type* data() const { return datavec_.data(); } + index1_type* data() { return datavec_.data(); } private: /// Initialize range data from sequences of lower and upper bounds @@ -90,15 +85,15 @@ class Range { /// \param lower_bound The lower bound of the range /// \param upper_bound The upper bound of the range /// \pre Assume \c rank_ is initialized to the rank of the range and - /// \c data_ has been allocated to hold 4*rank_ elements - /// \post \c data_ and \c volume_ are initialized with range dimension + /// \c datavec_ has been allocated to hold 4*rank_ elements + /// \post \c datavec_ and \c volume_ are initialized with range dimension /// information. template && detail::is_integral_range_v>> void init_range_data(const Index1& lower_bound, const Index2& upper_bound) { // Construct temp pointers - auto* MADNESS_RESTRICT const lower = data_; + auto* MADNESS_RESTRICT const lower = data(); auto* MADNESS_RESTRICT const upper = lower + rank_; auto* MADNESS_RESTRICT const extent = upper + rank_; auto* MADNESS_RESTRICT const stride = extent + rank_; @@ -145,15 +140,15 @@ class Range { /// \tparam PairRange Type representing a range of generalized pairs (see TiledArray::detail::is_gpair_v ) /// \param bounds The {lower,upper} bound of the range for each dimension /// \pre Assume \c rank_ is initialized to the rank of the range and - /// \c data_ has been allocated to hold 4*rank_ elements - /// \post \c data_ and \c volume_ are initialized with range dimension + /// \c datavec_ has been allocated to hold 4*rank_ elements + /// \post \c datavec_ and \c volume_ are initialized with range dimension /// information. // clang-format on template >> void init_range_data(const PairRange& bounds) { // Construct temp pointers - auto* MADNESS_RESTRICT const lower = data_; + auto* MADNESS_RESTRICT const lower = data(); auto* MADNESS_RESTRICT const upper = lower + rank_; auto* MADNESS_RESTRICT const extent = upper + rank_; auto* MADNESS_RESTRICT const stride = extent + rank_; @@ -187,14 +182,14 @@ class Range { /// \tparam Index An integral range type /// \param extents A sequence of extents for each dimension /// \pre Assume \c rank_ is initialized to the rank of the range and - /// \c data_ has been allocated to hold 4*rank_ elements - /// \post \c data_ and \c volume_ are initialized with range dimension + /// \c datavec_ has been allocated to hold 4*rank_ elements + /// \post \c datavec_ and \c volume_ are initialized with range dimension /// information. template >* = nullptr> void init_range_data(const Index& extents) { // Construct temp pointers - auto* MADNESS_RESTRICT const lower = data_; + auto* MADNESS_RESTRICT const lower = data(); auto* MADNESS_RESTRICT const upper = lower + rank_; auto* MADNESS_RESTRICT const extent = upper + rank_; auto* MADNESS_RESTRICT const stride = extent + rank_; @@ -226,8 +221,8 @@ class Range { /// \tparam Indices A pack of integral types /// \param extents A tuple of extents for each dimension /// \pre Assume \c rank_ is initialized to the rank of the range and - /// \c data_ has been allocated to hold 4*rank_ elements - /// \post \c data_ and \c volume_ are initialized with range dimension + /// \c datavec_ has been allocated to hold 4*rank_ elements + /// \post \c datavec_ and \c volume_ are initialized with range dimension /// information. template (extents); - auto* MADNESS_RESTRICT const lower = data_; + auto* MADNESS_RESTRICT const lower = data(); auto* MADNESS_RESTRICT const upper = lower + rank_; auto* MADNESS_RESTRICT const extent = upper + rank_; auto* MADNESS_RESTRICT const stride = extent + rank_; @@ -276,8 +271,8 @@ class Range { /// \param other_lower_bound The lower bound of the unpermuted range /// \param other_upper_bound The upper bound of the unpermuted range /// \pre Assume \c rank_ is initialized to the rank of the range and - /// \c data_ has been allocated to hold 4*rank_ elements - /// \post \c data_, \c offset_, and \c volume_ are initialized with the + /// \c datavec_ has been allocated to hold 4*rank_ elements + /// \post \c datavec_, \c offset_, and \c volume_ are initialized with the /// permuted range dimension information from \c other_lower_bound and /// \c other_upper_bound. void init_range_data( @@ -285,7 +280,7 @@ class Range { const index1_type* MADNESS_RESTRICT const other_lower_bound, const index1_type* MADNESS_RESTRICT const other_upper_bound) { // Create temporary pointers to this range data - auto* MADNESS_RESTRICT const lower = data_; + auto* MADNESS_RESTRICT const lower = data(); auto* MADNESS_RESTRICT const upper = lower + rank_; auto* MADNESS_RESTRICT const extent = upper + rank_; auto* MADNESS_RESTRICT const stride = extent + rank_; @@ -583,7 +578,6 @@ class Range { Range(const Range_& other) { if (other.rank_ > 0ul) { datavec_ = other.datavec_; - data_ = datavec_.data(); offset_ = other.offset_; volume_ = other.volume_; rank_ = other.rank_; @@ -595,11 +589,11 @@ class Range { /// \param other The range to be copied Range(Range_&& other) : datavec_(std::move(other.datavec_)), - data_(datavec_.data()), offset_(other.offset_), volume_(other.volume_), rank_(other.rank_) { - other.data_ = nullptr; + other.datavec_.clear(); + other.datavec_.shrink_to_fit(); other.offset_ = 0ul; other.volume_ = 0ul; other.rank_ = 0u; @@ -621,7 +615,6 @@ class Range { } else { // Simple copy will do datavec_ = other.datavec_; - data_ = datavec_.data(); offset_ = other.offset_; volume_ = other.volume_; } @@ -629,7 +622,7 @@ class Range { } /// Destructor - ~Range() { data_ = nullptr; } + ~Range() = default; /// Copy assignment operator @@ -637,7 +630,6 @@ class Range { /// \return A reference to this object Range_& operator=(const Range_& other) { datavec_ = other.datavec_; - data_ = datavec_.data(); rank_ = other.rank_; offset_ = other.offset_; volume_ = other.volume_; @@ -652,12 +644,12 @@ class Range { /// \throw nothing Range_& operator=(Range_&& other) { datavec_ = std::move(other.datavec_); - data_ = datavec_.data(); offset_ = other.offset_; volume_ = other.volume_; rank_ = other.rank_; - other.data_ = nullptr; + other.datavec_.clear(); + other.datavec_.shrink_to_fit(); other.offset_ = 0l; other.volume_ = 0ul; other.rank_ = 0u; From 69d0ee79398d6c23890252d37cfcbdc31fecd574 Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sun, 20 Dec 2020 13:23:27 -0500 Subject: [PATCH 03/12] - Range for ranks up to 8 avoids heap - rank-0 Range is supported (minimally tested) --- src/TiledArray/block_range.h | 22 +++++----- src/TiledArray/range.h | 85 +++++++++++++++++++----------------- tests/range.cpp | 41 ++++++++++------- 3 files changed, 81 insertions(+), 67 deletions(-) diff --git a/src/TiledArray/block_range.h b/src/TiledArray/block_range.h index cb2ab25336..0af22d3074 100644 --- a/src/TiledArray/block_range.h +++ b/src/TiledArray/block_range.h @@ -33,7 +33,7 @@ namespace TiledArray { /// Range that references a subblock of another range class BlockRange : public Range { private: - using Range::data_; + using Range::datavec_; using Range::offset_; using Range::rank_; using Range::volume_; @@ -58,10 +58,10 @@ class BlockRange : public Range { // Construct temp pointers const auto* MADNESS_RESTRICT const range_stride = range.stride_data(); - auto* MADNESS_RESTRICT const lower = data_; - auto* MADNESS_RESTRICT const upper = lower + rank_; - auto* MADNESS_RESTRICT const extent = upper + rank_; - auto* MADNESS_RESTRICT const stride = extent + rank_; + auto* MADNESS_RESTRICT const lower = lobound_data_nc(); + auto* MADNESS_RESTRICT const upper = upbound_data_nc(); + auto* MADNESS_RESTRICT const extent = extent_data_nc(); + auto* MADNESS_RESTRICT const stride = stride_data_nc(); // initialize bounds and extents auto lower_it = std::begin(lower_bound); @@ -110,10 +110,10 @@ class BlockRange : public Range { // Construct temp pointers const auto* MADNESS_RESTRICT const range_stride = range.stride_data(); - auto* MADNESS_RESTRICT const lower = data_; - auto* MADNESS_RESTRICT const upper = lower + rank_; - auto* MADNESS_RESTRICT const extent = upper + rank_; - auto* MADNESS_RESTRICT const stride = extent + rank_; + auto* MADNESS_RESTRICT const lower = lobound_data_nc(); + auto* MADNESS_RESTRICT const upper = upbound_data_nc(); + auto* MADNESS_RESTRICT const extent = extent_data_nc(); + auto* MADNESS_RESTRICT const stride = stride_data_nc(); // Compute range data int d = 0; @@ -333,8 +333,8 @@ class BlockRange : public Range { ordinal_type result = 0ul; // Get pointers to the data - const auto* MADNESS_RESTRICT const size = data_ + rank_ + rank_; - const auto* MADNESS_RESTRICT const stride = size + rank_; + const auto* MADNESS_RESTRICT const size = extent_data(); + const auto* MADNESS_RESTRICT const stride = stride_data(); // Compute the coordinate index of o in range. for (int i = int(rank_) - 1; i >= 0; --i) { diff --git a/src/TiledArray/range.h b/src/TiledArray/range.h index a00d60412a..8626376818 100644 --- a/src/TiledArray/range.h +++ b/src/TiledArray/range.h @@ -77,6 +77,11 @@ class Range { const index1_type* data() const { return datavec_.data(); } index1_type* data() { return datavec_.data(); } + index1_type* lobound_data_nc() { return data(); } + index1_type* upbound_data_nc() { return data() + rank_; } + index1_type* extent_data_nc() { return data() + (rank_<<1); } + index1_type* stride_data_nc() { return extent_data_nc() + rank_; } + private: /// Initialize range data from sequences of lower and upper bounds @@ -315,7 +320,7 @@ class Range { public: /// Default constructor - /// Construct a range that has zero rank, volume, and size. + /// Constructs a null range, i.e., it has zero volume and rank. Range() {} /// Construct range defined by upper and lower bound ranges @@ -409,6 +414,8 @@ class Range { rank_ = n; init_range_data(extent); } + else // rank-0 Range has unit volume + volume_ = 1; } /// Range constructor from an initializer list of extents @@ -432,6 +439,8 @@ class Range { rank_ = n; init_range_data(extent); } + else // rank=0 Range has unit volume + volume_ = 1; } // clang-format off @@ -474,6 +483,8 @@ class Range { rank_ = n; init_range_data(bounds); } + else // rank=0 Range has unit volume + volume_ = 1; } // clang-format off @@ -505,6 +516,8 @@ class Range { rank_ = n; init_range_data(bounds); } + else // rank=0 Range has unit volume + volume_ = 1; } // clang-format off @@ -538,6 +551,8 @@ class Range { rank_ = n; init_range_data(bounds); } + else // rank=0 Range has unit volume + volume_ = 1; } /// Range constructor from a pack of extents for each dimension @@ -575,14 +590,7 @@ class Range { /// Copy Constructor /// \param other The range to be copied - Range(const Range_& other) { - if (other.rank_ > 0ul) { - datavec_ = other.datavec_; - offset_ = other.offset_; - volume_ = other.volume_; - rank_ = other.rank_; - } - } + Range(const Range_& other) : datavec_(other.datavec_), offset_(other.offset_), volume_(other.volume_), rank_(other.rank_) {} /// Copy Constructor @@ -592,6 +600,7 @@ class Range { offset_(other.offset_), volume_(other.volume_), rank_(other.rank_) { + // put other into null state other.datavec_.clear(); other.datavec_.shrink_to_fit(); other.offset_ = 0ul; @@ -619,6 +628,8 @@ class Range { volume_ = other.volume_; } } + else // handle null and rank-0 case + volume_ = other.volume_; } /// Destructor @@ -648,6 +659,7 @@ class Range { volume_ = other.volume_; rank_ = other.rank_; + // put other into null state other.datavec_.clear(); other.datavec_.shrink_to_fit(); other.offset_ = 0l; @@ -657,6 +669,11 @@ class Range { return *this; } + /// Conversion to bool + + /// \return false if is null state, i.e. \c this->volume()==0 + explicit operator bool() const { return volume() != 0; } + /// Rank accessor /// \return The rank (number of dimensions) of this range @@ -676,7 +693,7 @@ class Range { /// \return A pointer to the lower bound data (see Range::lobound() ) /// \throw nothing - const index1_type* lobound_data() const { return data_; } + const index1_type* lobound_data() const { return data(); } /// Range lower bound accessor @@ -700,7 +717,7 @@ class Range { /// \return A pointer to the upper bound data (see Range::upbound() ) /// \throw nothing - const index1_type* upbound_data() const { return data_ + rank_; } + const index1_type* upbound_data() const { return data() + rank_; } /// Range upper bound accessor @@ -724,7 +741,7 @@ class Range { /// \return A pointer to the extent data (see Range::extent() ) /// \throw nothing - const index1_type* extent_data() const { return data_ + (rank_ + rank_); } + const index1_type* extent_data() const { return data() + (rank_<<1); } /// Range extent accessor @@ -748,7 +765,7 @@ class Range { /// \return A pointer to the stride data (see Range::stride() ) /// \throw nothing const index1_type* stride_data() const { - return data_ + (rank_ + rank_ + rank_); + return extent_data() + rank_; } /// Range stride accessor @@ -771,7 +788,7 @@ class Range { /// Range volume accessor - /// \return The total number of elements in the range. + /// \return The total number of elements in the range, or 0 if this is a null Range /// \throw nothing ordinal_type volume() const { return volume_; } @@ -794,7 +811,7 @@ class Range { /// \return An iterator that holds the lower bound index of a tensor (unless /// it has zero volume, then it returns same result as end()) \throw nothing const_iterator begin() const { - return (volume_ > 0) ? const_iterator(data_, this) : end(); + return (volume_ > 0) ? const_iterator(lobound_data(), this) : end(); } /// Index iterator factory @@ -803,7 +820,7 @@ class Range { /// the data layout of a row-major tensor. /// \return An iterator that holds the upper bound element index of a tensor /// \throw nothing - const_iterator end() const { return const_iterator(data_ + rank_, this); } + const_iterator end() const { return const_iterator(upbound_data(), this); } /// Check the coordinate to make sure it is within the range. @@ -817,8 +834,8 @@ class Range { typename std::enable_if, bool>::type* = nullptr> bool includes(const Index& index) const { - const auto* MADNESS_RESTRICT const lower = data_; - const auto* MADNESS_RESTRICT const upper = lower + rank_; + const auto* MADNESS_RESTRICT const lower = lobound_data(); + const auto* MADNESS_RESTRICT const upper = upbound_data(); bool result = (rank_ > 0u); unsigned int d = 0; @@ -922,8 +939,8 @@ class Range { template >> Range_& inplace_shift(const Index& bound_shift) { - auto* MADNESS_RESTRICT const lower = data_; - auto* MADNESS_RESTRICT const upper = data_ + rank_; + auto* MADNESS_RESTRICT const lower = lobound_data_nc(); + auto* MADNESS_RESTRICT const upper = upbound_data_nc(); const auto* MADNESS_RESTRICT const stride = upper + rank_ + rank_; // update the data @@ -1013,7 +1030,7 @@ class Range { ordinal_type ordinal(const Index& index) const { TA_ASSERT(includes(index)); - auto* MADNESS_RESTRICT const stride = data_ + rank_ + rank_ + rank_; + auto* MADNESS_RESTRICT const stride = stride_data(); ordinal_type result = 0ul; unsigned int d = 0; @@ -1074,8 +1091,8 @@ class Range { // Get pointers to the data auto* MADNESS_RESTRICT const result_data = result.data(); - const auto* MADNESS_RESTRICT const lower = data_; - const auto* MADNESS_RESTRICT const size = data_ + rank_ + rank_; + const auto* MADNESS_RESTRICT const lower = lobound_data(); + const auto* MADNESS_RESTRICT const size = extent_data(); // Compute the coordinate index of index in range. for (int i = int(rank_) - 1; i >= 0; --i) { @@ -1107,26 +1124,14 @@ class Range { return i; } - template ::value>::type* = nullptr> - void serialize(const Archive& ar) { - ar& rank_& datavec_& offset_& volume_; - data_ = datavec_.data(); - } - - template ::value>::type* = nullptr> - void serialize(const Archive& ar) const { + template + void serialize(Archive& ar) { ar& rank_& datavec_& offset_& volume_; } void swap(Range_& other) { // Get temp data std::swap(datavec_, other.datavec_); - data_ = datavec_.data(); - other.data_ = other.datavec_.data(); std::swap(offset_, other.offset_); std::swap(volume_, other.volume_); std::swap(rank_, other.rank_); @@ -1167,8 +1172,8 @@ class Range { void increment(index_type& i) const { TA_ASSERT(includes(i)); - const auto* MADNESS_RESTRICT const lower = data_; - const auto* MADNESS_RESTRICT const upper = data_ + rank_; + const auto* MADNESS_RESTRICT const lower = lobound_data(); + const auto* MADNESS_RESTRICT const upper = upbound_data(); for (int d = int(rank_) - 1; d >= 0; --d) { // increment coordinate @@ -1224,7 +1229,7 @@ inline Range& Range::operator*=(const Permutation& perm) { // Copy the lower and upper bound data into a temporary array container::svector temp_lower(rank_ << 1); const auto* MADNESS_RESTRICT const temp_upper = temp_lower.data() + rank_; - std::memcpy(temp_lower.data(), data_, (sizeof(index1_type) << 1) * rank_); + std::copy(lobound_data(), lobound_data() + (rank_<<1), temp_lower.data()); init_range_data(perm, temp_lower.data(), temp_upper); } diff --git a/tests/range.cpp b/tests/range.cpp index 9a745b0835..6c0c3052a4 100644 --- a/tests/range.cpp +++ b/tests/range.cpp @@ -85,23 +85,33 @@ BOOST_AUTO_TEST_CASE(constructors) { // Default Constructor BOOST_REQUIRE_NO_THROW(Range r0); Range r0; + BOOST_CHECK(!r0); BOOST_CHECK_EQUAL(r0.rank(), 0u); - BOOST_CHECK(r0.lobound_data() == nullptr); - BOOST_CHECK(r0.upbound_data() == nullptr); - BOOST_CHECK(r0.extent_data() == nullptr); - BOOST_CHECK(r0.stride_data() == nullptr); + BOOST_CHECK(r0.upbound_data() == r0.lobound_data()); + BOOST_CHECK(r0.extent_data() == r0.lobound_data()); + BOOST_CHECK(r0.stride_data() == r0.lobound_data()); BOOST_CHECK_EQUAL(r0.volume(), 0ul); // Copy of a default-constructed object BOOST_CHECK_NO_THROW(Range r00(r0)); Range r00(r0); + BOOST_CHECK(!r00); BOOST_CHECK_EQUAL(r00.rank(), 0u); - BOOST_CHECK(r00.lobound_data() == nullptr); - BOOST_CHECK(r00.upbound_data() == nullptr); - BOOST_CHECK(r00.extent_data() == nullptr); - BOOST_CHECK(r00.stride_data() == nullptr); + BOOST_CHECK(r00.upbound_data() == r00.lobound_data()); + BOOST_CHECK(r00.extent_data() == r00.lobound_data()); + BOOST_CHECK(r00.stride_data() == r00.lobound_data()); BOOST_CHECK_EQUAL(r00.volume(), 0ul); + // Rank-0 Range != null Range + BOOST_REQUIRE_NO_THROW(Range r0(std::vector{})); + Range r1(std::vector{}); + BOOST_CHECK(r1); + BOOST_CHECK_EQUAL(r1.rank(), 0u); + BOOST_CHECK(r1.upbound_data() == r1.lobound_data()); + BOOST_CHECK(r1.extent_data() == r1.lobound_data()); + BOOST_CHECK(r1.stride_data() == r1.lobound_data()); + BOOST_CHECK_EQUAL(r1.volume(), 1ul); + index f2 = finish; for (std::size_t i = 0; i < f2.size(); ++i) f2[i] += p2[i]; @@ -112,6 +122,7 @@ BOOST_AUTO_TEST_CASE(constructors) { BOOST_REQUIRE_NO_THROW(Range r5(1u, 2, 3ul, 4l)); // uses param pack BOOST_REQUIRE_NO_THROW(Range r5({0, 0, 0})); // zero extents are OK Range r5(p5); + BOOST_CHECK(r5); BOOST_CHECK_EQUAL_COLLECTIONS( r5.lobound_data(), r5.lobound_data() + r5.rank(), p0.begin(), p0.end()); BOOST_CHECK_EQUAL_COLLECTIONS( @@ -638,10 +649,9 @@ BOOST_AUTO_TEST_CASE(swap) { // Check that r contains the data of empty_range. BOOST_CHECK_EQUAL(r.rank(), 0u); - BOOST_CHECK(r.lobound_data() == nullptr); - BOOST_CHECK(r.upbound_data() == nullptr); - BOOST_CHECK(r.extent_data() == nullptr); - BOOST_CHECK(r.stride_data() == nullptr); + BOOST_CHECK(r.upbound_data() == r.lobound_data()); + BOOST_CHECK(r.extent_data() == r.lobound_data()); + BOOST_CHECK(r.stride_data() == r.lobound_data()); BOOST_CHECK_EQUAL(r.volume(), 0ul); // Check that empty_range contains the data of r. @@ -661,10 +671,9 @@ BOOST_AUTO_TEST_CASE(swap) { // Check that empty_range contains its original data. BOOST_CHECK_EQUAL(empty_range.rank(), 0u); - BOOST_CHECK(empty_range.lobound_data() == nullptr); - BOOST_CHECK(empty_range.upbound_data() == nullptr); - BOOST_CHECK(empty_range.extent_data() == nullptr); - BOOST_CHECK(empty_range.stride_data() == nullptr); + BOOST_CHECK(empty_range.upbound_data() == empty_range.lobound_data()); + BOOST_CHECK(empty_range.extent_data() == empty_range.lobound_data()); + BOOST_CHECK(empty_range.stride_data() == empty_range.lobound_data()); BOOST_CHECK_EQUAL(empty_range.volume(), 0ul); // Check that r its original data. From f987e86beaaa852202cff39fd1e574b4fb40378d Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sun, 20 Dec 2020 13:24:23 -0500 Subject: [PATCH 04/12] BlockRange UT's sped up by limiting the number of attempts to construct BlockRange --- tests/block_range.cpp | 44 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/block_range.cpp b/tests/block_range.cpp index 4f70592cae..8d8205e220 100644 --- a/tests/block_range.cpp +++ b/tests/block_range.cpp @@ -49,17 +49,34 @@ const Range BlockRangeFixture::r(std::array{{0, 1, 2}}, BOOST_FIXTURE_TEST_SUITE(block_range_suite, BlockRangeFixture, TA_UT_SKIP_IF_DISTRIBUTED) +const auto target_count = 20; + BOOST_AUTO_TEST_CASE(block_zero_lower_bound) { BlockRange block_range; + auto count_valid = 0; + auto count_invalid = 0; + auto skip = []() { + return GlobalFixture::world->rand() % 20 > 9; + }; + + // loop over all possible subblocks, skipping randomly, until target_count valid+invalid block ranges have been considered for (auto lower_it = r0.begin(); lower_it != r0.end(); ++lower_it) { const auto lower = *lower_it; for (auto upper_it = r0.begin(); upper_it != r0.end(); ++upper_it) { + if (skip()) continue; + if (count_valid == target_count && count_invalid == target_count) { + goto end; + } + auto upper = *upper_it; for (unsigned int i = 0u; i < upper.size(); ++i) ++(upper[i]); if (std::equal(lower.begin(), lower.end(), upper.begin(), [](std::size_t l, std::size_t r0) { return l < r0; })) { + if (count_valid == target_count) continue; + ++count_valid; + // Check that the sub-block is constructed without exceptions BOOST_CHECK_NO_THROW(block_range = BlockRange(r0, lower, upper)); @@ -95,25 +112,46 @@ BOOST_AUTO_TEST_CASE(block_zero_lower_bound) { } #ifdef TA_EXCEPTION_ERROR else { + if (count_invalid == target_count) continue; + ++count_invalid; + // Check for exception with invalid input BOOST_CHECK_THROW(BlockRange(r0, lower, upper), TiledArray::Exception); } +#else // TA_EXCEPTION_ERROR + count_invalid = target_count; #endif // TA_EXCEPTION_ERROR } } + end: ; } BOOST_AUTO_TEST_CASE(block) { BlockRange block_range; + auto count_valid = 0; + auto count_invalid = 0; + auto skip = []() { + return GlobalFixture::world->rand() % 20 > 9; + }; + + // loop over all possible subblocks, skipping randomly, until target_count valid+invalid block ranges have been considered for (auto lower_it = r.begin(); lower_it != r.end(); ++lower_it) { const auto lower = *lower_it; for (auto upper_it = r.begin(); upper_it != r.end(); ++upper_it) { + if (skip()) continue; + if (count_valid == target_count && count_invalid == target_count) { + goto end; + } + auto upper = *upper_it; for (unsigned int i = 0u; i < r.rank(); ++i) ++(upper[i]); if (std::equal(lower.begin(), lower.end(), upper.begin(), [](std::size_t l, std::size_t r) { return l < r; })) { + if (count_valid == target_count) continue; + ++count_valid; + // Check that the sub-block is constructed without exceptions BOOST_CHECK_NO_THROW(block_range = BlockRange(r, lower, upper)); @@ -229,12 +267,18 @@ BOOST_AUTO_TEST_CASE(block) { } #ifdef TA_EXCEPTION_ERROR else { + if (count_invalid == target_count) continue; + ++count_invalid; + // Check for exception with invalid input BOOST_CHECK_THROW(BlockRange(r, lower, upper), TiledArray::Exception); } +#else // TA_EXCEPTION_ERROR + count_invalid = target_count; #endif // TA_EXCEPTION_ERROR } } + end: ; } BOOST_AUTO_TEST_SUITE_END() From 9a8b9a26eac38115993a50699c0be36014720a9d Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sun, 20 Dec 2020 13:38:55 -0500 Subject: [PATCH 05/12] introduced TA_MAX_SOO_RANK_METADATA CMake variable to control the use of Small Object Optimization for metadata. The default is 8. --- CMakeLists.txt | 3 +++ INSTALL.md | 1 + src/TiledArray/config.h.in | 3 +++ src/TiledArray/range.h | 4 ++-- src/TiledArray/util/vector.h | 3 ++- 5 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6519eeec91..e6e91d1e6f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -133,6 +133,9 @@ option(TA_EXPERT "TiledArray Expert mode: disables automatically downloading or option(TA_SIGNED_1INDEX_TYPE "Enables the use of signed 1-index coordinate type (OFF in 1.0.0-alpha.2 and older)" ON) add_feature_info(SIGNED_1INDEX_TYPE TA_SIGNED_1INDEX_TYPE "Use of signed 1-index coordinate type in TiledArray") +# define this as needed +set(TA_MAX_SOO_RANK_METADATA 8 CACHE STRING "Specifies the max rank for which small object optimization will be used (hence, heap use avoided) for metadata objects") + option(TA_TRACE_TASKS "Enable debug tracing of MADNESS tasks in (some components of) TiledArray" OFF) add_feature_info(TASK_TRACE_DEBUG TA_TRACE_TASKS "Debug tracing of MADNESS tasks in (some components of) TiledArray") set(TILEDARRAY_ENABLE_TASK_DEBUG_TRACE ${TA_TRACE_TASKS}) diff --git a/INSTALL.md b/INSTALL.md index e6b1d349be..80a4f4aacd 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -366,6 +366,7 @@ support may be added. * `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. +* `TA_MAX_SOO_RANK_METADATA` -- Specifies the maximum rank for which to use Small Object Optimization (hence, avoid the use of the heap) for metadata. The default is `8`. # Build TiledArray diff --git a/src/TiledArray/config.h.in b/src/TiledArray/config.h.in index 57104cca9a..f1e8f4fdf2 100644 --- a/src/TiledArray/config.h.in +++ b/src/TiledArray/config.h.in @@ -152,6 +152,9 @@ # define TA_1INDEX_TYPE std::size_t #endif +/* Specifies the rank for up to which to use Small Object Optimization for metadata (e.g. Range, Range::index, etc.) */ +#cmakedefine TA_MAX_SOO_RANK_METADATA @TA_MAX_SOO_RANK_METADATA@ + /* Enables tracing MADNESS tasks in TiledArray */ #cmakedefine TILEDARRAY_ENABLE_TASK_DEBUG_TRACE 1 diff --git a/src/TiledArray/range.h b/src/TiledArray/range.h index 8626376818..731cf2eb2a 100644 --- a/src/TiledArray/range.h +++ b/src/TiledArray/range.h @@ -60,7 +60,7 @@ class Range { static_assert(detail::is_range_v); // index is a Range protected: - container::svector datavec_; + container::svector datavec_; ///< An array that holds the dimension information of the ///< range. The layout of the array is: ///< \code @@ -1227,7 +1227,7 @@ inline Range& Range::operator*=(const Permutation& perm) { TA_ASSERT(perm.size() == rank_); if (rank_ > 1ul) { // Copy the lower and upper bound data into a temporary array - container::svector temp_lower(rank_ << 1); + container::svector temp_lower(rank_ << 1); const auto* MADNESS_RESTRICT const temp_upper = temp_lower.data() + rank_; std::copy(lobound_data(), lobound_data() + (rank_<<1), temp_lower.data()); diff --git a/src/TiledArray/util/vector.h b/src/TiledArray/util/vector.h index 09ab5f8f3e..5d578ceb82 100644 --- a/src/TiledArray/util/vector.h +++ b/src/TiledArray/util/vector.h @@ -26,6 +26,7 @@ #ifndef TILEDARRAY_UTIL_VECTOR_H #define TILEDARRAY_UTIL_VECTOR_H +#include "TiledArray/config.h" #include #include @@ -38,7 +39,7 @@ namespace container { template using vector = std::vector; -template +template using svector = boost::container::small_vector; template From 69fa6933c77d8912e807225425fb2b171b43bad1 Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sun, 20 Dec 2020 13:50:20 -0500 Subject: [PATCH 06/12] amended TiledRange UT for updated semantics of Range::entent_data() (i.e. no longer necessarily null for null Range) --- tests/tiled_range.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tiled_range.cpp b/tests/tiled_range.cpp index ea88bcb077..8bacde44d6 100644 --- a/tests/tiled_range.cpp +++ b/tests/tiled_range.cpp @@ -38,8 +38,8 @@ BOOST_AUTO_TEST_CASE(constructor) { BOOST_REQUIRE_NO_THROW(TiledRange r0); TiledRange r0; std::vector s0(3, 0); - BOOST_CHECK(r0.tiles_range().extent_data() == nullptr); - BOOST_CHECK(r0.elements_range().extent_data() == nullptr); + BOOST_CHECK(!r0.tiles_range()); + BOOST_CHECK(!r0.elements_range()); } // check ranges constructor From b7a6d313156c713d14241d2ef259f37f0fc5c00e Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sun, 20 Dec 2020 13:52:48 -0500 Subject: [PATCH 07/12] dox++ --- src/TiledArray/range.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/TiledArray/range.h b/src/TiledArray/range.h index 731cf2eb2a..4b875f6c68 100644 --- a/src/TiledArray/range.h +++ b/src/TiledArray/range.h @@ -692,6 +692,7 @@ class Range { /// Range lower bound data accessor /// \return A pointer to the lower bound data (see Range::lobound() ) + /// \note Not necessarily nullptr for rank-0 or null Range /// \throw nothing const index1_type* lobound_data() const { return data(); } @@ -716,6 +717,7 @@ class Range { /// Range upper bound data accessor /// \return A pointer to the upper bound data (see Range::upbound() ) + /// \note Not necessarily nullptr for rank-0 or null Range /// \throw nothing const index1_type* upbound_data() const { return data() + rank_; } @@ -740,6 +742,7 @@ class Range { /// Range extent data accessor /// \return A pointer to the extent data (see Range::extent() ) + /// \note Not necessarily nullptr for rank-0 or null Range /// \throw nothing const index1_type* extent_data() const { return data() + (rank_<<1); } @@ -763,6 +766,7 @@ class Range { /// Range stride data accessor /// \return A pointer to the stride data (see Range::stride() ) + /// \note Not necessarily nullptr for rank-0 or null Range /// \throw nothing const index1_type* stride_data() const { return extent_data() + rank_; From f05a9caa5ce92b9c71e429f4eb3df44c59b6066d Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Sun, 20 Dec 2020 20:27:56 -0500 Subject: [PATCH 08/12] another ctor supports rank 0 --- src/TiledArray/range.h | 49 +++++++++++++++++++++--------------------- tests/range.cpp | 14 ++++++++++-- 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/TiledArray/range.h b/src/TiledArray/range.h index 4b875f6c68..8f23a80b50 100644 --- a/src/TiledArray/range.h +++ b/src/TiledArray/range.h @@ -60,7 +60,7 @@ class Range { static_assert(detail::is_range_v); // index is a Range protected: - container::svector datavec_; + container::svector datavec_; ///< An array that holds the dimension information of the ///< range. The layout of the array is: ///< \code @@ -79,7 +79,7 @@ class Range { index1_type* lobound_data_nc() { return data(); } index1_type* upbound_data_nc() { return data() + rank_; } - index1_type* extent_data_nc() { return data() + (rank_<<1); } + index1_type* extent_data_nc() { return data() + (rank_ << 1); } index1_type* stride_data_nc() { return extent_data_nc() + rank_; } private: @@ -355,7 +355,8 @@ class Range { init_datavec(n); rank_ = n; init_range_data(lower_bound, upper_bound); - } + } else // rank-0 Range has unit volume + volume_ = 1; } // clang-format off @@ -389,7 +390,8 @@ class Range { rank_ = n; if (n) { init_range_data(lower_bound, upper_bound); - } + } else // rank-0 Range has unit volume + volume_ = 1; } /// Range constructor from a range of extents @@ -413,8 +415,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(extent); - } - else // rank-0 Range has unit volume + } else // rank-0 Range has unit volume volume_ = 1; } @@ -438,8 +439,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(extent); - } - else // rank=0 Range has unit volume + } else // rank=0 Range has unit volume volume_ = 1; } @@ -482,8 +482,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(bounds); - } - else // rank=0 Range has unit volume + } else // rank=0 Range has unit volume volume_ = 1; } @@ -515,8 +514,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(bounds); - } - else // rank=0 Range has unit volume + } else // rank=0 Range has unit volume volume_ = 1; } @@ -550,8 +548,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(bounds); - } - else // rank=0 Range has unit volume + } else // rank=0 Range has unit volume volume_ = 1; } @@ -590,7 +587,11 @@ class Range { /// Copy Constructor /// \param other The range to be copied - Range(const Range_& other) : datavec_(other.datavec_), offset_(other.offset_), volume_(other.volume_), rank_(other.rank_) {} + Range(const Range_& other) + : datavec_(other.datavec_), + offset_(other.offset_), + volume_(other.volume_), + rank_(other.rank_) {} /// Copy Constructor @@ -627,8 +628,7 @@ class Range { offset_ = other.offset_; volume_ = other.volume_; } - } - else // handle null and rank-0 case + } else // handle null and rank-0 case volume_ = other.volume_; } @@ -744,7 +744,7 @@ class Range { /// \return A pointer to the extent data (see Range::extent() ) /// \note Not necessarily nullptr for rank-0 or null Range /// \throw nothing - const index1_type* extent_data() const { return data() + (rank_<<1); } + const index1_type* extent_data() const { return data() + (rank_ << 1); } /// Range extent accessor @@ -768,9 +768,7 @@ class Range { /// \return A pointer to the stride data (see Range::stride() ) /// \note Not necessarily nullptr for rank-0 or null Range /// \throw nothing - const index1_type* stride_data() const { - return extent_data() + rank_; - } + const index1_type* stride_data() const { return extent_data() + rank_; } /// Range stride accessor @@ -792,8 +790,8 @@ class Range { /// Range volume accessor - /// \return The total number of elements in the range, or 0 if this is a null Range - /// \throw nothing + /// \return The total number of elements in the range, or 0 if this is a null + /// Range \throw nothing ordinal_type volume() const { return volume_; } /// alias to volume() to conform to the TWG specification @@ -1231,9 +1229,10 @@ inline Range& Range::operator*=(const Permutation& perm) { TA_ASSERT(perm.size() == rank_); if (rank_ > 1ul) { // Copy the lower and upper bound data into a temporary array - container::svector temp_lower(rank_ << 1); + container::svector temp_lower( + rank_ << 1); const auto* MADNESS_RESTRICT const temp_upper = temp_lower.data() + rank_; - std::copy(lobound_data(), lobound_data() + (rank_<<1), temp_lower.data()); + std::copy(lobound_data(), lobound_data() + (rank_ << 1), temp_lower.data()); init_range_data(perm, temp_lower.data(), temp_upper); } diff --git a/tests/range.cpp b/tests/range.cpp index 6c0c3052a4..0e33dd742b 100644 --- a/tests/range.cpp +++ b/tests/range.cpp @@ -103,7 +103,7 @@ BOOST_AUTO_TEST_CASE(constructors) { BOOST_CHECK_EQUAL(r00.volume(), 0ul); // Rank-0 Range != null Range - BOOST_REQUIRE_NO_THROW(Range r0(std::vector{})); + BOOST_REQUIRE_NO_THROW(Range r1(std::vector{})); Range r1(std::vector{}); BOOST_CHECK(r1); BOOST_CHECK_EQUAL(r1.rank(), 0u); @@ -112,6 +112,16 @@ BOOST_AUTO_TEST_CASE(constructors) { BOOST_CHECK(r1.stride_data() == r1.lobound_data()); BOOST_CHECK_EQUAL(r1.volume(), 1ul); + // another Rank-0 Range + BOOST_CHECK_NO_THROW(Range r11(std::vector{}, std::vector{})); + Range r11(std::vector{}, std::vector{}); + BOOST_CHECK(r11); + BOOST_CHECK_EQUAL(r11.rank(), 0u); + BOOST_CHECK(r11.upbound_data() == r11.lobound_data()); + BOOST_CHECK(r11.extent_data() == r11.lobound_data()); + BOOST_CHECK(r11.stride_data() == r11.lobound_data()); + BOOST_CHECK_EQUAL(r11.volume(), 1ul); + index f2 = finish; for (std::size_t i = 0; i < f2.size(); ++i) f2[i] += p2[i]; @@ -144,7 +154,7 @@ BOOST_AUTO_TEST_CASE(constructors) { #ifdef TA_EXCEPTION_ERROR BOOST_CHECK_THROW(Range r2(f2, p2), Exception); // lobound > upbound -#endif // TA_EXCEPTION_ERROR +#endif // TA_EXCEPTION_ERROR Range r2(p2, f2); BOOST_CHECK_EQUAL_COLLECTIONS( r2.lobound_data(), r2.lobound_data() + r2.rank(), p2.begin(), p2.end()); From ba7870171abf64ac2dcb1335ad47fbbf6d1a4e90 Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Mon, 21 Dec 2020 10:09:02 -0500 Subject: [PATCH 09/12] stepped on same rank-0-as-valid-state rakes again ... reverted, added warning to Range class dox --- src/TiledArray/range.h | 45 +++++++++++++++++++++--------------------- tests/btas.cpp | 12 +++++++++++ tests/range.cpp | 12 +++++------ 3 files changed, 40 insertions(+), 29 deletions(-) diff --git a/src/TiledArray/range.h b/src/TiledArray/range.h index 8f23a80b50..abf808f42e 100644 --- a/src/TiledArray/range.h +++ b/src/TiledArray/range.h @@ -28,15 +28,21 @@ namespace TiledArray { /// \brief A (hyperrectangular) interval on \f$ Z^n \f$, space of integer -/// n-indices - -/// This object represents an n-dimensional, hyperrectangular array -/// of integers. It provides information on the rank (number of dimensions), -/// (nonnegative) lower bound, upper bound, extent (size), and stride of -/// each dimension. It can also be used to -/// test if an element is included in the range with a coordinate index or -/// ordinal offset. Finally, it can be used to convert coordinate indices to -/// ordinal offsets and vice versa. +/// \f$ n \f$-indices + +/// This object is a range of integers on a \f$ n \f$-dimensional, +/// hyperrectangular domain, where _rank_ (aka order, number of dimensions) +/// \f$ n>0 \f$. It is fully specified by its lower and upper bounds. It also +/// provides zero-cost access to +/// _extent_ (size) and _stride_ for +/// each _mode_ (dimension). Range is a _regular_ type with null default state. +/// \warning Range is with rank 0 is _null_, i.e. invalid. There are many +/// reasons that rank-0 Range is not supported; summarily, rank-0 Range is not a +/// special case of rank-\f$ n \f$ Range as many invariants of nonzero-rank +/// Range are not observed by it. E.g. for any nonempty nonzero-rank Range the +/// lower bound differs from its upper bound. To define the 0-dimensional limit +/// of array/tensor to be a scalar, the volume of rank-0 Range should be 1, but +/// clearly its lower and upper bounds are equal. class Range { public: typedef Range Range_; ///< This object type @@ -355,8 +361,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(lower_bound, upper_bound); - } else // rank-0 Range has unit volume - volume_ = 1; + } // rank-0 Range is null } // clang-format off @@ -390,8 +395,7 @@ class Range { rank_ = n; if (n) { init_range_data(lower_bound, upper_bound); - } else // rank-0 Range has unit volume - volume_ = 1; + } // rank-0 Range is null } /// Range constructor from a range of extents @@ -415,8 +419,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(extent); - } else // rank-0 Range has unit volume - volume_ = 1; + } // rank-0 Range is null } /// Range constructor from an initializer list of extents @@ -439,8 +442,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(extent); - } else // rank=0 Range has unit volume - volume_ = 1; + } // rank-0 Range is null } // clang-format off @@ -482,8 +484,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(bounds); - } else // rank=0 Range has unit volume - volume_ = 1; + } // rank-0 Range is null } // clang-format off @@ -514,8 +515,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(bounds); - } else // rank=0 Range has unit volume - volume_ = 1; + } // rank-0 Range is null } // clang-format off @@ -548,8 +548,7 @@ class Range { init_datavec(n); rank_ = n; init_range_data(bounds); - } else // rank=0 Range has unit volume - volume_ = 1; + } // rank-0 Range is null } /// Range constructor from a pack of extents for each dimension diff --git a/tests/btas.cpp b/tests/btas.cpp index df9122695a..ac33e6ad57 100644 --- a/tests/btas.cpp +++ b/tests/btas.cpp @@ -232,6 +232,18 @@ BTASFixture::TArrayDB0& BTASFixture::array(size_t idx) { BOOST_FIXTURE_TEST_SUITE(btas_suite, BTASFixture) +BOOST_AUTO_TEST_CASE_TEMPLATE(tensor_ctor, Tensor, tensor_types) { + BOOST_REQUIRE_NO_THROW(Tensor{}); + Tensor t0; + BOOST_CHECK(t0.empty()); + + // copy of empty Tensor should be empty ... this makes sure that Tensor's + // range treats rank 0 as null state, not as rank-0 state with volume 1 + BOOST_REQUIRE_NO_THROW(Tensor t1 = t0); + Tensor t1 = t0; + BOOST_CHECK(t1.empty()); +} + BOOST_AUTO_TEST_CASE_TEMPLATE(copy, Array, array_types) { const auto& a = array(0); TArrayD b; diff --git a/tests/range.cpp b/tests/range.cpp index 0e33dd742b..d591e0cd3b 100644 --- a/tests/range.cpp +++ b/tests/range.cpp @@ -102,25 +102,25 @@ BOOST_AUTO_TEST_CASE(constructors) { BOOST_CHECK(r00.stride_data() == r00.lobound_data()); BOOST_CHECK_EQUAL(r00.volume(), 0ul); - // Rank-0 Range != null Range + // Rank-0 Range *IS* null Range BOOST_REQUIRE_NO_THROW(Range r1(std::vector{})); Range r1(std::vector{}); - BOOST_CHECK(r1); + BOOST_CHECK(!r1); BOOST_CHECK_EQUAL(r1.rank(), 0u); BOOST_CHECK(r1.upbound_data() == r1.lobound_data()); BOOST_CHECK(r1.extent_data() == r1.lobound_data()); BOOST_CHECK(r1.stride_data() == r1.lobound_data()); - BOOST_CHECK_EQUAL(r1.volume(), 1ul); + BOOST_CHECK_EQUAL(r1.volume(), 0ul); - // another Rank-0 Range + // another way to make Rank-0 Range BOOST_CHECK_NO_THROW(Range r11(std::vector{}, std::vector{})); Range r11(std::vector{}, std::vector{}); - BOOST_CHECK(r11); + BOOST_CHECK(!r11); BOOST_CHECK_EQUAL(r11.rank(), 0u); BOOST_CHECK(r11.upbound_data() == r11.lobound_data()); BOOST_CHECK(r11.extent_data() == r11.lobound_data()); BOOST_CHECK(r11.stride_data() == r11.lobound_data()); - BOOST_CHECK_EQUAL(r11.volume(), 1ul); + BOOST_CHECK_EQUAL(r11.volume(), 0ul); index f2 = finish; for (std::size_t i = 0; i < f2.size(); ++i) f2[i] += p2[i]; From c81dfaac1398f6f40bce644599b2ca1f7b4ed683 Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Mon, 21 Dec 2020 10:10:22 -0500 Subject: [PATCH 10/12] dox cleanup --- src/TiledArray/range.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/TiledArray/range.h b/src/TiledArray/range.h index abf808f42e..c6e465f675 100644 --- a/src/TiledArray/range.h +++ b/src/TiledArray/range.h @@ -66,15 +66,15 @@ class Range { static_assert(detail::is_range_v); // index is a Range protected: + /// A vector that holds the dimension information of the + /// range. Its layout: + /// \code + /// { lobound[0], ..., lobound[rank_ - 1], + /// upbound[0], ..., upbound[rank_ - 1], + /// extent[0], ..., extent[rank_ - 1], + /// stride[0], ..., stride[rank_ - 1] } + /// \endcode container::svector datavec_; - ///< An array that holds the dimension information of the - ///< range. The layout of the array is: - ///< \code - ///< { lobound[0], ..., lobound[rank_ - 1], - ///< upbound[0], ..., upbound[rank_ - 1], - ///< extent[0], ..., extent[rank_ - 1], - ///< stride[0], ..., stride[rank_ - 1] } - ///< \endcode distance_type offset_ = 0l; ///< Ordinal index offset correction ordinal_type volume_ = 0ul; ///< Total number of elements unsigned int rank_ = 0u; ///< The rank (or number of dimensions) in the range @@ -592,7 +592,7 @@ class Range { volume_(other.volume_), rank_(other.rank_) {} - /// Copy Constructor + /// Move Constructor /// \param other The range to be copied Range(Range_&& other) From acf89fa3204333a143b6b7019e8a75781dd0f4ff Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Mon, 21 Dec 2020 10:15:31 -0500 Subject: [PATCH 11/12] Range default copy ctor/assignment are now OK --- src/TiledArray/range.h | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/TiledArray/range.h b/src/TiledArray/range.h index c6e465f675..1438f11c2b 100644 --- a/src/TiledArray/range.h +++ b/src/TiledArray/range.h @@ -586,11 +586,7 @@ class Range { /// Copy Constructor /// \param other The range to be copied - Range(const Range_& other) - : datavec_(other.datavec_), - offset_(other.offset_), - volume_(other.volume_), - rank_(other.rank_) {} + Range(const Range_& other) = default; /// Move Constructor @@ -638,14 +634,7 @@ class Range { /// \param other The range to be copied /// \return A reference to this object - Range_& operator=(const Range_& other) { - datavec_ = other.datavec_; - rank_ = other.rank_; - offset_ = other.offset_; - volume_ = other.volume_; - - return *this; - } + Range_& operator=(const Range_& other) = default; /// Move assignment operator From 06060b7842381434189e2143d1508f345b7eb071 Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Mon, 21 Dec 2020 10:15:50 -0500 Subject: [PATCH 12/12] Added Range move ctor/assignment UTs --- tests/range.cpp | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/range.cpp b/tests/range.cpp index d591e0cd3b..afbac08106 100644 --- a/tests/range.cpp +++ b/tests/range.cpp @@ -346,6 +346,25 @@ BOOST_AUTO_TEST_CASE(constructors) { BOOST_CHECK_EQUAL(r4.volume(), volume); } +BOOST_AUTO_TEST_CASE(move_constructor) { + Range r_copy(r); + Range x(std::move(r_copy)); + + // Check that the data in x matches that of r. + BOOST_CHECK_EQUAL_COLLECTIONS(x.lobound_data(), x.lobound_data() + x.rank(), + r.lobound_data(), r.lobound_data() + r.rank()); + BOOST_CHECK_EQUAL_COLLECTIONS(x.upbound_data(), x.upbound_data() + x.rank(), + r.upbound_data(), r.upbound_data() + r.rank()); + BOOST_CHECK_EQUAL_COLLECTIONS(x.extent_data(), x.extent_data() + x.rank(), + r.extent_data(), r.extent_data() + r.rank()); + BOOST_CHECK_EQUAL_COLLECTIONS(x.stride_data(), x.stride_data() + x.rank(), + r.stride_data(), r.stride_data() + r.rank()); + BOOST_CHECK_EQUAL(x.volume(), r.volume()); + + // moved-from object is null + BOOST_CHECK(!r_copy); +} + BOOST_AUTO_TEST_CASE(assignment_operator) { Range x; x = r; @@ -362,6 +381,26 @@ BOOST_AUTO_TEST_CASE(assignment_operator) { BOOST_CHECK_EQUAL(x.volume(), r.volume()); } +BOOST_AUTO_TEST_CASE(move_assignment_operator) { + Range r_copy(r); + Range x; + x = std::move(r_copy); + + // Check that the data in x matches that of r. + BOOST_CHECK_EQUAL_COLLECTIONS(x.lobound_data(), x.lobound_data() + x.rank(), + r.lobound_data(), r.lobound_data() + r.rank()); + BOOST_CHECK_EQUAL_COLLECTIONS(x.upbound_data(), x.upbound_data() + x.rank(), + r.upbound_data(), r.upbound_data() + r.rank()); + BOOST_CHECK_EQUAL_COLLECTIONS(x.extent_data(), x.extent_data() + x.rank(), + r.extent_data(), r.extent_data() + r.rank()); + BOOST_CHECK_EQUAL_COLLECTIONS(x.stride_data(), x.stride_data() + x.rank(), + r.stride_data(), r.stride_data() + r.rank()); + BOOST_CHECK_EQUAL(x.volume(), r.volume()); + + // moved-from object is null + BOOST_CHECK(!r_copy); +} + BOOST_AUTO_TEST_CASE(ostream) { std::stringstream stm; stm << "[ " << start << ", " << finish << " )";