diff --git a/dwave/optimization/include/dwave-optimization/array.hpp b/dwave/optimization/include/dwave-optimization/array.hpp index f4124b05..0340e49a 100644 --- a/dwave/optimization/include/dwave-optimization/array.hpp +++ b/dwave/optimization/include/dwave-optimization/array.hpp @@ -363,10 +363,10 @@ struct Update { class Array { public: /// A std::random_access_iterator over the values in the array. - using iterator = BufferIterator; + using iterator = BufferIterator; /// A std::random_access_iterator over the values in the array. - using const_iterator = BufferIterator; + using const_iterator = BufferIterator; template using cache_type = std::unordered_map; @@ -374,7 +374,7 @@ class Array { template using optional_cache_type = std::optional>>; - using View = std::ranges::subrange; + using View = std::ranges::subrange; /// Constant used to signal that the size is based on the state. static constexpr ssize_t DYNAMIC_SIZE = -1; @@ -436,16 +436,25 @@ class Array { // Interface methods ****************************************************** /// Return an iterator to the beginning of the array. - const_iterator begin(const State& state) const { - if (contiguous()) return const_iterator(buff(state)); + auto begin(const State& state) const { + if (ndim() == 0) { + // A 0d iterator is pretty ill-defined, so we return a 1d one. + // The iterator doesn't manage the lifespan of the shape/strides + // so we need them to be static here. + static constexpr ssize_t shape = 1; + static constexpr ssize_t strides = 0; + return const_iterator(buff(state), 1, &shape, &strides); + } return const_iterator(buff(state), ndim(), shape().data(), strides().data()); } - /// Return an iterator to the end of the array. - const_iterator end(const State& state) const { return this->begin(state) + this->size(state); } + /// Return a sentinel indicating the end of the array's state as a flattened array. + auto end(const State& state) const { return begin(state) + size(state); } /// Return a container-like view over the array. - const View view(const State& state) const { return View(begin(state), end(state)); } + const View view(const State& state) const { + return std::ranges::subrange(begin(state), end(state)); + } /// The number of doubles in the flattened array. /// If the size is dependent on the state, returns Array::DYNAMIC_SIZE. diff --git a/dwave/optimization/include/dwave-optimization/iterators.hpp b/dwave/optimization/include/dwave-optimization/iterators.hpp index 8974ee76..cac29325 100644 --- a/dwave/optimization/include/dwave-optimization/iterators.hpp +++ b/dwave/optimization/include/dwave-optimization/iterators.hpp @@ -29,135 +29,97 @@ concept shape_like = std::ranges::sized_range and std::integral`, then the iterator does not need to hold -/// any information. -/// If `From` is `void`, then the iterator does the interpretation of the buffer -/// at runtime. -/// -/// `IsConst` toggles whether the iterator is an output iterator or not. -/// `IsConst` must be `false` unless `To` is the same type as `From`. -template - requires(IsConst || std::same_as) +template + requires requires { + // Cannot create an output iterator when From != To + requires std::is_const::value or std::is_same::value; + } class BufferIterator { public: using difference_type = std::ptrdiff_t; - using pointer = std::conditional::type; - using reference = std::conditional::type; + using pointer = To*; + using reference = To&; using value_type = To; /// Default constructor. BufferIterator() = default; - /// Copy Constructor. + /// Copy constructor BufferIterator(const BufferIterator& other) noexcept : ptr_(other.ptr_), format_(other.format_), - shape_(other.shape_ ? std::make_unique(*other.shape_) : nullptr) {} + ndim_(other.ndim_), + shape_(other.shape_), + strides_(other.strides_), + loc_(std::make_unique_for_overwrite(ndim_)) { + assert(ndim_ >= 0); + std::copy(other.loc_.get(), other.loc_.get() + ndim_, loc_.get()); + } - /// Move Constructor. - BufferIterator(BufferIterator&& other) = default; + /// Move constructor + BufferIterator(BufferIterator&&) = default; - /// Construct a contiguous iterator pointing to `ptr` when `From` is `void`. - template - explicit BufferIterator(const T* ptr) noexcept - requires(!DType) - : ptr_(ptr), format_(format_of()), shape_() {} - - /// Construct a contiguous iterator pointing to ptr when the type of ptr is - /// known at compile-time. - explicit BufferIterator(const From* ptr) noexcept - requires(DType) - : ptr_(ptr), format_(), shape_() {} - explicit BufferIterator(From* ptr) noexcept - requires(DType && !IsConst) - : ptr_(ptr), format_(), shape_() {} + // todo: untemplated versions /// Construct a non-contiguous iterator from a shape/strides defined as /// observing pointers when `From` is `void`. template - BufferIterator(const T* ptr, - ssize_t ndim, // number of dimensions in the array - const ssize_t* shape, // shape of the array - const ssize_t* strides) // strides for each dimension of the array - noexcept - requires(!DType) + BufferIterator( + T* ptr, ssize_t ndim, const ssize_t* shape, + const ssize_t* strides) noexcept // todo: enforce agreement with From when needed : ptr_(ptr), format_(format_of()), - shape_(std::make_unique(ndim, shape, strides)) {} + ndim_(ndim), + shape_(shape), + strides_(strides), + loc_(std::make_unique(ndim_)) { + assert(ndim_ >= 0); + } - /// Construct a non-contiguous iterator from a shape/strides defined as - /// ranges when `From` is `void`. template - BufferIterator(const T* ptr, - const std::span shape, // shape of the array - const std::span strides) // strides of the array - requires(!DType) - : BufferIterator(ptr, shape.size(), shape.data(), strides.data()) { - assert(shape.size() == strides.size()); + BufferIterator(T* ptr, std::span shape, std::span strides) + : ptr_(ptr), + format_(format_of()), + ndim_(std::ranges::size(shape)), + shape_(shape.data()), + strides_(strides.data()), + loc_(std::make_unique(ndim_)) { + assert(ndim_ >= 0); + assert(std::ranges::size(shape) == std::ranges::size(strides)); } - /// Construct a non-contiguous iterator from a shape/strides defined as - /// observing pointers when `From` satisfies `DType`. - BufferIterator(pointer ptr, - ssize_t ndim, // number of dimensions in the array - const ssize_t* shape, // shape of the array - const ssize_t* strides) // strides for each dimension of the array - noexcept - requires(DType) - : ptr_(ptr), format_(), shape_(std::make_unique(ndim, shape, strides)) {} + BufferIterator& operator=(const BufferIterator& other) noexcept { + ptr_ = other.ptr_; + format_ = other.format_; - /// Construct a non-contiguous iterator from a shape/strides defined as - /// ranges when `From` satisfies `DType`. - BufferIterator(pointer ptr, - const std::span shape, // shape of the array - const std::span strides) // strides of the array - requires(DType) - : BufferIterator(ptr, shape.size(), shape.data(), strides.data()) { - assert(shape.size() == strides.size()); - } + // Reallocate the location only if we have a different ndim. + // We'll overwrite it shortly + if (ndim_ != other.ndim_) loc_ = std::make_unique(ndim_); - /// Copy and move assignment operators - BufferIterator& operator=(BufferIterator other) noexcept { - std::swap(this->ptr_, other.ptr_); - std::swap(this->format_, other.format_); - std::swap(this->shape_, other.shape_); - return *this; + ndim_ = other.ndim_; + shape_ = other.shape_; + strides_ = other.strides_; + std::copy(other.loc_.get(), other.loc_.get() + ndim_, loc_.get()); } + BufferIterator& operator=(BufferIterator&&) = default; + /// Destructor ~BufferIterator() = default; - /// Dereference the iterator when the iterator is an output iterator. - value_type& operator*() const noexcept - requires(std::same_as && !IsConst) + reference operator*() const noexcept + requires(std::same_as) { return *static_cast(ptr_); } - /// Dereference the iterator when it is not an output iterator, but To and From types - /// are the same. - const value_type& operator*() const noexcept - requires(std::same_as && IsConst) - { - return *static_cast(ptr_); - } - /// Dereference the iterator when the iterator is not an output iterator. value_type operator*() const noexcept - requires(!std::same_as && IsConst) + requires(not std::same_as) { if constexpr (DType) { // In this case we know at compile-time what type we're converting from - return *static_cast(ptr_); + return *ptr_; } else { // In this case we need to do some switch behavior at runtime switch (format_) { @@ -207,27 +169,18 @@ class BufferIterator { /// Increment the iterator by `rhs` steps. BufferIterator& operator+=(difference_type rhs) { - // We want to increment the pointer by a number of bytes, so we cast - // void* to char*. But we also need to respect the const-correctness. - using ptr_type = std::conditional::type; - - if (shape_) { - ptr_ = static_cast(ptr_) + shape_->increment(rhs); - } else { - ptr_ = static_cast(ptr_) + rhs * itemsize(); - } + // Increment our pointer by a number of bytes while maintaining const-correctness. + using ptr_type = std::conditional::value, const char*, char*>::type; + ptr_ = reinterpret_cast(reinterpret_cast(ptr_) + increment_(rhs)); return *this; } template BufferIterator& operator+=(Shape&& rhs) { - // We want to increment the pointer by a number of bytes, so we cast - // void* to char*. But we also need to respect the const-correctness. - using ptr_type = std::conditional::type; - - assert(shape_ && "only defined for shaped iterators"); - ptr_ = static_cast(ptr_) + shape_->advance(std::forward(rhs)); - + // Increment our pointer by a number of bytes while maintaining const-correctness. + using ptr_type = std::conditional::value, const char*, char*>::type; + ptr_ = reinterpret_cast(reinterpret_cast(ptr_) + + increment_(std::forward(rhs))); return *this; } BufferIterator& operator+=(std::initializer_list rhs) { @@ -237,44 +190,38 @@ class BufferIterator { /// Decrement the iterator by `rhs` steps. BufferIterator& operator-=(difference_type rhs) { return *this += -rhs; } - /// Return `true` is `lhs` and `rhs` are pointing to he same underlying - /// value. friend bool operator==(const BufferIterator& lhs, const BufferIterator& rhs) { - if (lhs.shape_ && rhs.shape_) { - return *lhs.shape_ == *rhs.shape_; - } else if (lhs.shape_ || rhs.shape_) { - // when one is shaped and the other is not (or they have different - // shapes/strides) then they are not mutually reachable and this - // method is not defined. - assert(false && "rhs must be reachable from lhs"); - unreachable(); - } else { - // both are contiguous, so they are equal if their pointers match. - return lhs.ptr_ == rhs.ptr_; - } + // Check that lhs and rhs are consistent with each other. + assert(std::ranges::equal(std::span(lhs.shape_, lhs.ndim_), + std::span(rhs.shape_, rhs.ndim_)) && + "lhs must be reachable from rhs but lhs and rhs do not have the same shape"); + assert(std::ranges::equal(std::span(lhs.strides_, lhs.ndim_), + std::span(rhs.strides_, rhs.ndim_)) && + "lhs must be reachable from rhs but lhs and rhs do not have the same strides"); + + return std::equal(lhs.loc_.get(), lhs.loc_.get() + lhs.ndim_, rhs.loc_.get()); } friend bool operator==(const BufferIterator& lhs, std::default_sentinel_t) { - assert(lhs.shape_ && "only defined for shaped iterators"); - assert(lhs.shape_->ndim > 0 && "only defined for iterators with ndim > 0"); - assert(lhs.shape_->shape[0] >= 0 && "only defined for non-dynamic shapes"); - return lhs.shape_->loc[0] >= lhs.shape_->shape[0]; + if (lhs.ndim_ <= 0) return true; + assert(lhs.shape_[0] >= 0 && "only defined for non-dynamic shapes"); + return lhs.loc_[0] >= lhs.shape_[0]; } /// Three-way comparison between two iterators. friend std::strong_ordering operator<=>(const BufferIterator& lhs, const BufferIterator& rhs) { - if (lhs.shape_ && rhs.shape_) { - return *lhs.shape_ <=> *rhs.shape_; - } else if (lhs.shape_ || rhs.shape_) { - // when one is shaped and the other is not (or they have different - // shapes/strides) then they are not mutually reachable and this - // method is not defined. - assert(false && "rhs must be reachable from lhs"); - unreachable(); - } else { - // both are contiguous, so they are equal if their pointers match. - return lhs.ptr_ <=> rhs.ptr_; + // Check that lhs and rhs are consistent with each other. + assert(std::ranges::equal(std::span(lhs.shape_, lhs.ndim_), + std::span(rhs.shape_, rhs.ndim_)) && + "lhs must be reachable from rhs but lhs and rhs do not have the same shape"); + assert(std::ranges::equal(std::span(lhs.strides_, lhs.ndim_), + std::span(rhs.strides_, rhs.ndim_)) && + "lhs must be reachable from rhs but lhs and rhs do not have the same strides"); + + for (ssize_t axis = 0; axis < lhs.ndim_; ++axis) { + if (lhs.loc_[axis] != rhs.loc_[axis]) return lhs.loc_[axis] <=> rhs.loc_[axis]; } + return std::strong_ordering::equal; } /// Return an iterator pointing to the location `rhs` steps from `lhs`. @@ -293,23 +240,42 @@ class BufferIterator { /// Return the number of times we would need to increment `rhs` in order to reach `lhs`. friend difference_type operator-(const BufferIterator& lhs, const BufferIterator& rhs) { - if (lhs.shape_ && rhs.shape_) { - return *lhs.shape_ - *rhs.shape_; - } else if (lhs.shape_ || rhs.shape_) { - // when one is shaped and the other is not (or they have different - // shapes/strides) then they are not mutually reachable and this - // method is not defined. - assert(false && "rhs must be reachable from lhs"); - unreachable(); - } else { - // sanity check that they have the same itemsize, otherwise this is not defined - assert(lhs.itemsize() == rhs.itemsize() && "rhs must be reachable from lhs"); + // Check that lhs and rhs are consistent with each other. + assert(std::ranges::equal(std::span(lhs.shape_, lhs.ndim_), + std::span(rhs.shape_, rhs.ndim_)) && + "lhs must be reachable from rhs but lhs and rhs do not have the same shape"); + assert(std::ranges::equal(std::span(lhs.strides_, lhs.ndim_), + std::span(rhs.strides_, rhs.ndim_)) && + "lhs must be reachable from rhs but lhs and rhs do not have the same strides"); + + // calculate the difference in steps based on the respective locs + difference_type difference = 0; + difference_type step = 1; + for (ssize_t axis = lhs.ndim_ - 1; axis >= 0; --axis) { + difference += (lhs.loc_[axis] - rhs.loc_[axis]) * step; + step *= lhs.shape_[axis]; + } - // both are contiguous, so it's just the distance between pointers - // but we need to be careful! Because it's type dependent - return (static_cast(lhs.ptr_) - static_cast(rhs.ptr_)) / - lhs.itemsize(); + return difference; + } + + friend difference_type operator-(std::default_sentinel_t, const BufferIterator& rhs) { + // how many steps from rhs to the end + + if (rhs.ndim_ <= 0) return 0; + + difference_type difference = 0; + difference_type step = 1; + for (ssize_t axis = rhs.ndim_ - 1; axis > 0; --axis) { + difference -= rhs.loc_[axis] * step; + step *= rhs.shape_[axis]; } + difference += (rhs.shape_[0] - rhs.loc_[0]) * step; + + return difference; + } + friend difference_type operator-(const BufferIterator& lhs, std::default_sentinel_t) { + return -(std::default_sentinel - lhs); } /// The number of bytes used to encode values in the buffer. @@ -343,202 +309,106 @@ class BufferIterator { /// Return the location in the shaped array that the iterator currently /// points to. - /// Only defined for shaped iterators. - std::span location() { - assert(shape_ && "only defined for shaped iterators"); - return std::span(shape_->loc.get(), shape_->ndim); - } - - /// Return `true` if the iterator is shaped. - bool shaped() const noexcept { return static_cast(shape_); } + std::span location() { return std::span(loc_.get(), ndim_); } private: - struct ShapeInfo { - ShapeInfo() = default; - - // Copy constructor - ShapeInfo(const ShapeInfo& other) noexcept - : ndim(other.ndim), - shape(other.shape), - strides(other.strides), - loc(std::make_unique(ndim)) { - std::copy(other.loc.get(), other.loc.get() + ndim, loc.get()); - } - - // Move Constructor - ShapeInfo(ShapeInfo&& other) = default; - - ShapeInfo(ssize_t ndim, const ssize_t* shape, const ssize_t* strides) noexcept - : ndim(ndim), - shape(shape), - strides(strides), - loc(std::make_unique(ndim)) { - std::fill(loc.get(), loc.get() + ndim, 0); - } - - // Copy and move assignment operators - ShapeInfo& operator=(ShapeInfo other) noexcept { - std::swap(ndim, other.ndim); - std::swap(shape, other.shape); - std::swap(strides, other.strides); - std::swap(loc, other.loc); - return *this; - } - - ~ShapeInfo() = default; - - // Check if another ShapeInfo is in the same location. For performance we - // only check the location when debug symbols are off. - friend bool operator==(const ShapeInfo& lhs, const ShapeInfo& rhs) { - // Check that lhs and rhs are consistent with each other. - assert(std::ranges::equal(std::span(lhs.shape, lhs.ndim), - std::span(rhs.shape, rhs.ndim)) && - "lhs must be reachable from rhs but lhs and rhs do not have the same shape"); - assert(std::ranges::equal(std::span(lhs.strides, lhs.ndim), - std::span(rhs.strides, rhs.ndim)) && - "lhs must be reachable from rhs but lhs and rhs do not have the same strides"); - - return std::equal(lhs.loc.get(), lhs.loc.get() + lhs.ndim, rhs.loc.get()); - } - friend std::strong_ordering operator<=>(const ShapeInfo& lhs, const ShapeInfo& rhs) { - // Check that lhs and rhs are consistent with each other. - assert(std::ranges::equal(std::span(lhs.shape, lhs.ndim), - std::span(rhs.shape, rhs.ndim)) && - "lhs must be reachable from rhs but lhs and rhs do not have the same shape"); - assert(std::ranges::equal(std::span(lhs.strides, lhs.ndim), - std::span(rhs.strides, rhs.ndim)) && - "lhs must be reachable from rhs but lhs and rhs do not have the same strides"); - - for (ssize_t axis = 0; axis < lhs.ndim; ++axis) { - if (lhs.loc[axis] != rhs.loc[axis]) return lhs.loc[axis] <=> rhs.loc[axis]; - } - return std::strong_ordering::equal; - } - - friend difference_type operator-(const ShapeInfo& lhs, const ShapeInfo& rhs) { - // Check that lhs and rhs are consistent with each other. - assert(std::ranges::equal(std::span(lhs.shape, lhs.ndim), - std::span(rhs.shape, rhs.ndim)) && - "lhs must be reachable from rhs but lhs and rhs do not have the same shape"); - assert(std::ranges::equal(std::span(lhs.strides, lhs.ndim), - std::span(rhs.strides, rhs.ndim)) && - "lhs must be reachable from rhs but lhs and rhs do not have the same strides"); - - // calculate the difference in steps based on the respective locs - difference_type difference = 0; - difference_type step = 1; - for (ssize_t axis = lhs.ndim - 1; axis >= 0; --axis) { - difference += (lhs.loc[axis] - rhs.loc[axis]) * step; - step *= lhs.shape[axis]; - } - - return difference; + // TODO: doc + std::ptrdiff_t increment_(shape_like auto&& multi_increment) { + assert(static_cast(std::ranges::size(multi_increment)) == ndim_); + + std::ptrdiff_t offset = 0; // the number of bytes we need to move + + for (ssize_t axis = 0; axis < ndim_; ++axis) { + // Make sure the new loc is in range, otherwise this function is undefined! + assert([&]() { + if (axis == 0) return true; // we allow out of bounds on axis 0 + ssize_t new_loc = loc_[axis] + multi_increment[axis]; + return 0 <= new_loc and new_loc < shape_[axis]; + }()); + + offset += strides_[axis] * multi_increment[axis]; + loc_[axis] += multi_increment[axis]; } - std::ptrdiff_t advance(shape_like auto&& multi_increment) { - assert(static_cast(std::ranges::size(multi_increment)) == ndim); - - std::ptrdiff_t offset = 0; // the number of bytes we need to move - - for (ssize_t axis = 0; axis < ndim; ++axis) { - // Make sure the new loc is in range, otherwise this function is undefined! - assert([&]() { - if (axis == 0) return true; // we allow out of bounds on axis 0 - ssize_t new_loc = loc[axis] + multi_increment[axis]; - return 0 <= new_loc and new_loc < shape[axis]; - }()); + return offset; + } - offset += strides[axis] * multi_increment[axis]; - loc[axis] += multi_increment[axis]; + // Return the pointer offset (in bytes) relative to the current + // position in order to increment the iterator n times. + // n can be negative. + // This method *does* update the loc_ but does not increment the pointer + // TODO: make that clearer + std::ptrdiff_t increment_(const ssize_t n = 1) { + if (n == 0) return 0; + if (ndim_ == 0) return 0; // incrementing a scalar does nothing + + assert(ndim_ > 0); + + // working from right-to-left, figure out how many steps in each + // axis. We handle axis 0 as a special case + + std::ptrdiff_t offset = 0; // the number of bytes we need to move + + // We'll be using std::div() over ssize_t, so we'll store our + // current location in the struct returned by it. + // Unfortunately std::div() is not templated, but overloaded, + // so we use decltype instead. + decltype(std::div(ssize_t(), ssize_t())) qr{.quot = n, .rem = 0}; + + for (ssize_t axis = ndim_ - 1; axis >= 1; --axis) { + // A bit of sanity checking on our location + // We can go "beyond" the relevant memory on the 0th axis, but + // otherwise the location must be nonnegative and strictly less + // than the size in that dimension. + assert(0 <= loc_[axis]); + assert(axis == 0 || loc_[axis] < shape_[axis]); + + // if we're partway through the axis, we shift to + // the beginning by adding the number of steps to the total + // that we want to go, and updating the offset accordingly + if (loc_[axis]) { + qr.quot += loc_[axis]; + offset -= loc_[axis] * strides_[axis]; + // loc_[axis] = 0; // overwritten later, so skip resetting the loc } - return offset; - } + // now, the number of steps might be more than our axis + // can support, so we do a div + qr = std::div(qr.quot, shape_[axis]); - // Return the pointer offset (in bytes) relative to the current - // position in order to increment the iterator n times. - // n can be negative. - std::ptrdiff_t increment(const ssize_t n = 1) { - // handle a few simple cases with faster implementations - if (n == 0) return 0; - // if (n == +1) return increment1(); - // if (n == -1) return decrement1(); - - assert(ndim > 0); // we shouldn't be here if we're a scalar. - - // working from right-to-left, figure out how many steps in each - // axis. We handle axis 0 as a special case - - std::ptrdiff_t offset = 0; // the number of bytes we need to move - - // We'll be using std::div() over ssize_t, so we'll store our - // current location in the struct returned by it. - // Unfortunately std::div() is not templated, but overloaded, - // so we use decltype instead. - decltype(std::div(ssize_t(), ssize_t())) qr{.quot = n, .rem = 0}; - - for (ssize_t axis = this->ndim - 1; axis >= 1; --axis) { - // A bit of sanity checking on our location - // We can go "beyond" the relevant memory on the 0th axis, but - // otherwise the location must be nonnegative and strictly less - // than the size in that dimension. - assert(0 <= loc[axis]); - assert(axis == 0 || loc[axis] < shape[axis]); - - // if we're partway through the axis, we shift to - // the beginning by adding the number of steps to the total - // that we want to go, and updating the offset accordingly - if (loc[axis]) { - qr.quot += loc[axis]; - offset -= loc[axis] * strides[axis]; - // loc[axis] = 0; // overwritten later, so skip resetting the loc - } - - // now, the number of steps might be more than our axis - // can support, so we do a div - qr = std::div(qr.quot, shape[axis]); - - // adjust so that the remainder is positive - if (qr.rem < 0) { - qr.quot -= 1; - qr.rem += shape[axis]; - } - - // finally adjust our location and offset - loc[axis] = qr.rem; - offset += qr.rem * strides[axis]; - // qr.rem = 0; // overwritten later, so skip resetting the .rem - - // if there's nothing left to do then exit early - if (qr.quot == 0) return offset; + // adjust so that the remainder is positive + if (qr.rem < 0) { + qr.quot -= 1; + qr.rem += shape_[axis]; } - offset += qr.quot * strides[0]; - loc[0] += qr.quot; + // finally adjust our location and offset + loc_[axis] = qr.rem; + offset += qr.rem * strides_[axis]; + // qr.rem = 0; // overwritten later, so skip resetting the .rem - return offset; + // if there's nothing left to do then exit early + if (qr.quot == 0) return offset; } - // Information about the shape/strides of the parent array. Note - // the pointers are non-owning! - ssize_t ndim; - const ssize_t* shape; - const ssize_t* strides; + offset += qr.quot * strides_[0]; + loc_[0] += qr.quot; - // The current location of the iterator within the parent array. - std::unique_ptr loc; - }; + return offset; + } // If we're const, then hold a const void*, else hold void* - std::conditional::type ptr_ = nullptr; + From* ptr_ = nullptr; + + // A format character indicating what type we are. We store this regardless + // of whether the format is known at compile-time or not + FormatCharacter format_ = FormatCharacter::double_; - // If we know the type of the buffer at compile-time, we don't need to - // store any information about it. Otherwise we keep a FormatCharacter - // indicating what type is it. - struct empty {}; // no information stored - std::conditional, empty, FormatCharacter>::type format_; + ssize_t ndim_ = 0; + const ssize_t* shape_ = nullptr; + const ssize_t* strides_ = nullptr; - std::unique_ptr shape_ = nullptr; + std::unique_ptr loc_ = nullptr; }; } // namespace dwave::optimization diff --git a/dwave/optimization/include/dwave-optimization/typing.hpp b/dwave/optimization/include/dwave-optimization/typing.hpp index 4883c621..17b7764b 100644 --- a/dwave/optimization/include/dwave-optimization/typing.hpp +++ b/dwave/optimization/include/dwave-optimization/typing.hpp @@ -25,19 +25,19 @@ namespace dwave::optimization { /// See https://numpy.org/doc/stable/reference/arrays.dtypes.html for more /// information. template -concept DType = std::same_as || // np.float32 - std::same_as || // np.float64 - std::same_as || // np.bool - std::same_as || // np.int8 - std::same_as || // np.int16 - std::same_as || // np.int32 - std::same_as; // np.int64 +concept DType = std::same_as || // np.float32 + std::same_as || // np.float64 + std::same_as || // np.bool + std::same_as || // np.int8 + std::same_as || // np.int16 + std::same_as || // np.int32 + std::same_as; // np.int64 /// The `OptionalDType` concept is satisfied if and only if `T` is `void` /// or satisfies `DType`. /// `void` is used to signal that the dtype is unknown at compile-time. template -concept OptionalDType = DType || std::same_as; +concept OptionalDType = DType || std::same_as; // Dev note: We'd like to work with `DType` only, but in order to work with the // buffer protocol (https://docs.python.org/3/c-api/buffer.html) we need to @@ -62,14 +62,14 @@ enum class FormatCharacter : char { /// Get the format character associated with a supported DType. template constexpr FormatCharacter format_of() { - if constexpr (std::same_as) return FormatCharacter::float_; - if constexpr (std::same_as) return FormatCharacter::double_; - if constexpr (std::same_as) return FormatCharacter::bool_; - if constexpr (std::same_as) return FormatCharacter::int_; - if constexpr (std::same_as) return FormatCharacter::signedchar_; - if constexpr (std::same_as) return FormatCharacter::signedshort_; - if constexpr (std::same_as) return FormatCharacter::signedlong_; - if constexpr (std::same_as) return FormatCharacter::signedlonglong_; + if constexpr (std::same_as) return FormatCharacter::float_; + if constexpr (std::same_as) return FormatCharacter::double_; + if constexpr (std::same_as) return FormatCharacter::bool_; + if constexpr (std::same_as) return FormatCharacter::int_; + if constexpr (std::same_as) return FormatCharacter::signedchar_; + if constexpr (std::same_as) return FormatCharacter::signedshort_; + if constexpr (std::same_as) return FormatCharacter::signedlong_; + if constexpr (std::same_as) return FormatCharacter::signedlonglong_; } } // namespace dwave::optimization diff --git a/dwave/optimization/src/nodes/flow.cpp b/dwave/optimization/src/nodes/flow.cpp index 297eaa33..0615e822 100644 --- a/dwave/optimization/src/nodes/flow.cpp +++ b/dwave/optimization/src/nodes/flow.cpp @@ -72,8 +72,8 @@ std::span ExtractNode::diff(const State& state) const { } void ExtractNode::initialize_state(State& state) const { - const Array::View condition = condition_ptr_->view(state); - const Array::View arr = arr_ptr_->view(state); + const std::ranges::sized_range auto condition = condition_ptr_->view(state); + const std::ranges::sized_range auto arr = arr_ptr_->view(state); std::vector values; values.reserve(condition.size()); diff --git a/dwave/optimization/src/nodes/reduce.cpp b/dwave/optimization/src/nodes/reduce.cpp index e7ba0b49..b3111416 100644 --- a/dwave/optimization/src/nodes/reduce.cpp +++ b/dwave/optimization/src/nodes/reduce.cpp @@ -851,12 +851,8 @@ auto ReduceNode::reduce_(const State& state, const ssize_t index) cons // We can then create an iterator that iterates of the reduction group auto begin = array_ptr_->begin(state); - if (begin.shaped()) { - begin += multi_index; - } else { - begin += ravel_multi_index(multi_index, array_ptr_->shape()); - } - auto it = BufferIterator(&*begin, subspace_shape, subspace_strides); + begin += multi_index; + auto it = BufferIterator(&*begin, subspace_shape, subspace_strides); return ufunc.reduce(std::ranges::subrange(it, std::default_sentinel), initial); } diff --git a/tests/cpp/test_iterators.cpp b/tests/cpp/test_iterators.cpp index 9f67719d..55dfbb70 100644 --- a/tests/cpp/test_iterators.cpp +++ b/tests/cpp/test_iterators.cpp @@ -17,6 +17,9 @@ #include #include +#include + +#include "catch2/benchmark/catch_benchmark_all.hpp" #include "catch2/catch_template_test_macros.hpp" #include "catch2/catch_test_macros.hpp" #include "catch2/generators/catch_generators.hpp" @@ -27,6 +30,39 @@ using Catch::Matchers::RangeEquals; namespace dwave::optimization { +TEMPLATE_TEST_CASE("BufferIterator", "", // + float, double, // + bool, // + std::int8_t, std::int16_t, std::int32_t, std::int64_t) { + GIVEN("auto it = BufferIterator<...>()") { + auto it = BufferIterator(); + + CHECK(it == std::default_sentinel); + CHECK(std::default_sentinel == it); + CHECK_FALSE(it != std::default_sentinel); + CHECK_FALSE(std::default_sentinel != it); + + CHECK(it - std::default_sentinel == 0); + CHECK(std::default_sentinel - it == 0); + + CHECK(it == BufferIterator()); + CHECK(it <= BufferIterator()); + CHECK(it >= BufferIterator()); + CHECK_FALSE(it != BufferIterator()); + CHECK_FALSE(it < BufferIterator()); + CHECK_FALSE(it > BufferIterator()); + + CHECK(it - BufferIterator() == 0); + CHECK(BufferIterator() - it == 0); + + CHECK(it + 1 == it); // does nothing + + CHECK_THAT(it.location(), RangeEquals(std::vector{})); + } +} + +///////////////////////////////////////////////////// + // note: we don't test bool here because it's a bit of an edge case TEMPLATE_TEST_CASE("BufferIterator - templated", "", // float, double, std::int8_t, std::int16_t, std::int32_t, std::int64_t) { @@ -34,10 +70,12 @@ TEMPLATE_TEST_CASE("BufferIterator - templated", "", // GIVEN("A buffer of the given type") { std::vector buffer(10); std::iota(buffer.begin(), buffer.end(), 0); + std::array shape{10}; + std::array strides{sizeof(TestType)}; THEN("We can construct an iterator reading it as if it was doubles without specifying the " "buffer type at compile-time") { - auto it = BufferIterator(buffer.data()); + auto it = BufferIterator(buffer.data(), shape, strides); // the output of the iterator is double as requested static_assert(std::same_as); @@ -47,7 +85,7 @@ TEMPLATE_TEST_CASE("BufferIterator - templated", "", // THEN("We can construct an iterator reading it as if it was doubles specifying the buffer " "type at compile-time") { - auto it = BufferIterator(buffer.data()); + auto it = BufferIterator(buffer.data(), shape, strides); // the output of the iterator is double as requested if constexpr (std::same_as) { @@ -60,7 +98,7 @@ TEMPLATE_TEST_CASE("BufferIterator - templated", "", // } THEN("We can construct a non-const iterator reading as the same type") { - auto it = BufferIterator(buffer.data()); + auto it = BufferIterator(buffer.data(), shape, strides); // the output of the iterator is a non-const reference static_assert(std::same_as); @@ -76,9 +114,11 @@ TEMPLATE_TEST_CASE("BufferIterator - templated", "", // GIVEN("A buffer of doubles") { std::vector buffer(10); std::iota(buffer.begin(), buffer.end(), 0); + std::array shape{10}; + std::array strides{sizeof(double)}; THEN("We can construct an iterator reading it as the given type") { - auto it = BufferIterator(buffer.data()); + auto it = BufferIterator(buffer.data(), shape, strides); // the output of the iterator is TestType as requested static_assert(std::same_as); @@ -89,35 +129,39 @@ TEMPLATE_TEST_CASE("BufferIterator - templated", "", // THEN("We can construct a strided 1D iterator equivalent to buffer[::2]") { const ssize_t size = 5; const ssize_t stride = 2 * sizeof(double); - auto it = BufferIterator(buffer.data(), 1, &size, &stride); + auto it = BufferIterator(buffer.data(), 1, &size, &stride); CHECK_THAT(std::ranges::subrange(it, it + 5), RangeEquals({0, 2, 4, 6, 8})); } } } -TEST_CASE("BufferIterator - bool") { +TEST_CASE("BufferIterator old - bool") { GIVEN("An array of bools") { std::array buffer{false, true, false, false, true}; + std::array shape{5}; + std::array strides{sizeof(bool)}; - auto it = BufferIterator(buffer.data()); + auto it = BufferIterator(buffer.data(), shape, strides); CHECK_THAT(std::ranges::subrange(it, it + 5), RangeEquals({0, 1, 0, 0, 1})); } GIVEN("An array of doubles") { std::array buffer{0, 1, 2, 3, 0, 1, 2, 3, 0, 1}; + std::array shape{10}; + std::array strides{sizeof(double)}; - auto it = BufferIterator(buffer.data()); + auto it = BufferIterator(buffer.data(), shape, strides); CHECK_THAT(std::ranges::subrange(it, it + 10), RangeEquals({false, true, true, true, false, true, true, true, false, true})); } } -TEST_CASE("BufferIterator") { - using ArrayIterator = BufferIterator; - using ConstArrayIterator = BufferIterator; +TEST_CASE("BufferIterator - old") { + using ArrayIterator = BufferIterator; + using ConstArrayIterator = BufferIterator; static_assert(std::is_nothrow_constructible::value); static_assert(std::is_nothrow_constructible::value); @@ -207,9 +251,10 @@ TEST_CASE("BufferIterator") { } THEN("The shaped iterator can be used with a sentinel") { - CHECK(it.shaped()); CHECK(it != std::default_sentinel); CHECK(it + values.size() == std::default_sentinel); + CHECK(std::default_sentinel - it == values.size()); + CHECK(it - std::default_sentinel == -(std::default_sentinel - it)); CHECK_THAT(std::ranges::subrange(it, std::default_sentinel), RangeEquals(values)); } @@ -252,27 +297,6 @@ TEST_CASE("BufferIterator") { } } - WHEN("We make an ConstArrayIterator from values.data()") { - auto it = ConstArrayIterator(values.data()); - - THEN("It behaves like a contiguous iterator") { - CHECK(*it == 0); - ++it; - CHECK(*it == 1); - } - - THEN("We can do iterator arithmetic") { - CHECK(*(it + 0) == 0); - CHECK(*(it + 1) == 1); - CHECK(*(it + 2) == 2); - CHECK(*(it + 3) == 3); - CHECK(*(it + 4) == 4); - - CHECK(ConstArrayIterator(values.data() + 4) - it == 4); - CHECK(it - ConstArrayIterator(values.data() + 4) == -4); - } - } - WHEN("We specify a shape and strides defining a subset of the values") { std::vector shape{5}; std::vector strides{2 * sizeof(double)}; @@ -329,12 +353,12 @@ TEST_CASE("BufferIterator") { } } - THEN("We can construct another vector using reverse iterators") { - auto copy = std::vector(); - copy.assign(std::reverse_iterator(ConstArrayIterator(values.data()) + 6), - std::reverse_iterator(ConstArrayIterator(values.data()))); - CHECK_THAT(copy, RangeEquals({5, 4, 3, 2, 1, 0})); - } + // THEN("We can construct another vector using reverse iterators") { + // auto copy = std::vector(); + // copy.assign(std::reverse_iterator(ConstArrayIterator(values.data()) + 6), + // std::reverse_iterator(ConstArrayIterator(values.data()))); + // CHECK_THAT(copy, RangeEquals({5, 4, 3, 2, 1, 0})); + // } } }