diff --git a/cxx/include/tensorwrapper/buffer/buffer_base.hpp b/cxx/include/tensorwrapper/buffer/buffer_base.hpp index 1225cc23..f322b994 100644 --- a/cxx/include/tensorwrapper/buffer/buffer_base.hpp +++ b/cxx/include/tensorwrapper/buffer/buffer_base.hpp @@ -166,6 +166,23 @@ class BufferBase : public BufferBaseCommon, bool approximately_equal_(const BufferViewBase& rhs, double tol) const; + /** @brief Returns the element with the offsets specified by @p index. + * + * @param[in] index The offsets into each mode of *this for the desired + * element. + * + * @return A const reference to the element at the specified offsets. + */ + virtual const_element_reference get_elem_(index_vector index) const = 0; + + /** @brief Sets the specified element to @p new_value. + * + * @param[in] index The offsets into each mode of *this for the desired + * element. + * @param[in] new_value The new value for the specified element. + */ + virtual void set_elem_(index_vector index, element_type new_value) = 0; + private: template dsl_reference binary_op_common_(FxnType&& fxn, label_type this_labels, diff --git a/cxx/include/tensorwrapper/buffer/buffer_base_common.hpp b/cxx/include/tensorwrapper/buffer/buffer_base_common.hpp index e0bf60ce..ef2c718a 100644 --- a/cxx/include/tensorwrapper/buffer/buffer_base_common.hpp +++ b/cxx/include/tensorwrapper/buffer/buffer_base_common.hpp @@ -18,6 +18,8 @@ #include #include #include +#include +#include namespace tensorwrapper::buffer { @@ -45,6 +47,11 @@ class BufferBaseCommon { using layout_pointer = typename traits_type::layout_pointer; using const_layout_reference = typename traits_type::const_layout_reference; using rank_type = typename traits_type::rank_type; + using size_type = typename traits_type::size_type; + using element_type = typename traits_type::element_type; + using const_element_reference = + typename traits_type::const_element_reference; + using index_vector = typename traits_type::index_vector; ///@} // ------------------------------------------------------------------------- @@ -93,6 +100,91 @@ class BufferBaseCommon { return has_layout() ? layout().rank() : 0; } + /** @brief Returns the element with the offsets specified by @p index. + * + * @param[in] index The offsets into each mode of *this for the desired + * element. The length of @p index must equal rank(). + * + * @return A const reference to the element at the specified offsets. + * + * @throw std::out_of_range if the length of @p index does not equal + * rank() or if any entry in @p index is out of + * bounds. Strong throw guarantee. + * @throw std::runtime_error if *this does not support element access + * (e.g. *this is a view with no such + * implementation). Strong throw guarantee. + */ + const_element_reference get_element(index_vector index) const { + return derived_().get_elem_(std::move(index)); + } + + /** @brief Sets the element with the offsets specified by @p index to + * @p value. + * + * @param[in] index The offsets into each mode of *this for the desired + * element. The length of @p index must equal rank(). + * @param[in] value The new value for the specified element. + * + * @throw std::out_of_range if the length of @p index does not equal + * rank() or if any entry in @p index is out of + * bounds. Strong throw guarantee. + * @throw std::runtime_error if *this does not support element access + * (e.g. *this is a view with no such + * implementation). Strong throw guarantee. + */ + void set_element(index_vector index, element_type value) { + derived_().set_elem_(std::move(index), std::move(value)); + } + + /** @brief Returns the element with the offsets given by @p offsets. + * + * This overload allows the offsets to be provided as an arbitrary + * number of arguments (one per mode) instead of as an index_vector. It + * is implemented in terms of get_element(index_vector) const. + * + * @tparam Offsets The types of the offsets. Expected to be integral + * types implicitly convertible to size_type. + * + * @param[in] offsets The offsets into each mode of *this for the + * desired element. + * + * @return A const reference to the element at the specified offsets. + * + * @throw std::out_of_range if the number of offsets does not equal + * rank() or if any offset is out of bounds. + * Strong throw guarantee. + */ + template + const_element_reference get_element(Offsets... offsets) const { + return get_element(index_vector{static_cast(offsets)...}); + } + + /** @brief Sets the element with the offsets given by @p offsets to + * @p value. + * + * This overload allows the offsets to be provided as an arbitrary + * number of arguments (one per mode) instead of as an index_vector. It + * is implemented in terms of set_element(index_vector, element_type). + * + * @tparam Offsets The types of the offsets. Expected to be integral + * types implicitly convertible to size_type. + * + * @param[in] offsets The offsets into each mode of *this for the + * desired element. + * @param[in] value The new value for the specified element. + * + * @throw std::out_of_range if the number of offsets does not equal + * rank() or if any offset is out of bounds. + * Strong throw guarantee. + */ + template + void set_element(Args... args) { + static_assert(sizeof...(Args) >= 1, + "set_element requires at least a value argument"); + set_element_unpack_(std::make_index_sequence{}, + std::make_tuple(args...)); + } + // ------------------------------------------------------------------------- // -- Utility methods // ------------------------------------------------------------------------- @@ -150,6 +242,18 @@ class BufferBaseCommon { template friend class BufferBaseCommon; + /// Splits the last element of @p values off as the new value and + /// forwards the rest as the offsets for set_element(index_vector, + /// element_type). Used to implement the variadic overload of + /// set_element, since a function parameter pack can not be followed by + /// a deduced fixed parameter. + template + void set_element_unpack_(std::index_sequence, Tuple&& values) { + set_element( + index_vector{static_cast(std::get(values))...}, + static_cast(std::get(values))); + } + Derived& derived_() noexcept { return static_cast(*this); } /// Access derived for CRTP diff --git a/cxx/include/tensorwrapper/buffer/buffer_view_base.hpp b/cxx/include/tensorwrapper/buffer/buffer_view_base.hpp index 583530d3..c118cdcc 100644 --- a/cxx/include/tensorwrapper/buffer/buffer_view_base.hpp +++ b/cxx/include/tensorwrapper/buffer/buffer_view_base.hpp @@ -38,6 +38,11 @@ namespace tensorwrapper::buffer { * constructed or moved-from views have no layout (has_layout() is false, * layout() throws). * + * BufferViewBase is abstract: it declares get_elem_()/set_elem_() (required + * by BufferBaseCommon for get_element()/set_element()) as pure virtual, + * since it has no notion of how to access an aliased buffer's data on its + * own. Derived classes that do (e.g. ReplicatedView) must implement them. + * * @tparam BufferBaseType Either BufferBase or const BufferBase. */ template @@ -57,7 +62,10 @@ class BufferViewBase : public BufferBaseCommon> { using const_pimpl_reference = const pimpl_type&; public: + using typename my_base_type::const_element_reference; using typename my_base_type::const_layout_reference; + using typename my_base_type::element_type; + using typename my_base_type::index_vector; using typename my_base_type::layout_pointer; using typename my_base_type::layout_reference; using typename my_base_type::layout_type; @@ -152,6 +160,18 @@ class BufferViewBase : public BufferBaseCommon> { return !(*this == rhs); } + /** @brief Defaulted (virtual) dtor. + * + * BufferViewBase is made polymorphic so that derived classes (e.g. + * ReplicatedView) can override get_elem_()/set_elem_() and have that + * override reached even when called through the CRTP + * BufferBaseCommon>::get_element()/ + * set_element(), whose static type is frozen at BufferViewBase. + * + * @throw None No throw guarantee. + */ + virtual ~BufferViewBase() noexcept = default; + protected: friend my_base_type; friend class BufferBase; @@ -180,6 +200,26 @@ class BufferViewBase : public BufferBaseCommon> { return *this == rhs; } + /** @brief Implements the get_elem_() hook required by BufferBaseCommon. + * + * BufferViewBase itself has no notion of how to retrieve an element (it + * only aliases a layout, not a buffer's data), so this is left for + * derived classes that do have access to the aliased buffer's data + * (e.g. ReplicatedView) to implement. + * + * @param[in] index The offsets into each mode for the desired element. + */ + virtual const_element_reference get_elem_(index_vector index) const = 0; + + /** @brief Implements the set_elem_() hook required by BufferBaseCommon. + * + * @see get_elem_() for why this is left to derived classes. + * + * @param[in] index The offsets into each mode for the desired element. + * @param[in] value The new value for the specified element. + */ + virtual void set_elem_(index_vector index, element_type value) = 0; + private: void assert_pimpl_() const { if(!m_pimpl_) { diff --git a/cxx/include/tensorwrapper/buffer/replicated.hpp b/cxx/include/tensorwrapper/buffer/replicated.hpp index a99f772a..0b667367 100644 --- a/cxx/include/tensorwrapper/buffer/replicated.hpp +++ b/cxx/include/tensorwrapper/buffer/replicated.hpp @@ -39,12 +39,15 @@ class Replicated : public ReplicatedCommon, public Local { friend my_base_type; friend my_base_type::sliceable_base; - virtual const_element_reference get_elem_(index_vector index) const = 0; - virtual void set_elem_(index_vector index, element_type value) = 0; + using typename my_base_type::const_element_reference; + using typename my_base_type::element_reference; + using typename my_base_type::element_type; + using typename my_base_type::index_vector; + virtual slice_type slice_(index_vector first_elem, - index_vector last_elem) = 0; + index_vector last_elem) = 0; virtual const_slice_type slice_(index_vector first_elem, - index_vector last_elem) const = 0; + index_vector last_elem) const = 0; }; } // namespace tensorwrapper::buffer diff --git a/cxx/include/tensorwrapper/buffer/replicated_view.hpp b/cxx/include/tensorwrapper/buffer/replicated_view.hpp index 7eb932b1..2a9bb5da 100644 --- a/cxx/include/tensorwrapper/buffer/replicated_view.hpp +++ b/cxx/include/tensorwrapper/buffer/replicated_view.hpp @@ -159,10 +159,10 @@ class ReplicatedView friend typename common_base_type::sliceable_base; /// Implements get_elem for the view. - const_element_reference get_elem_(index_vector index) const; + const_element_reference get_elem_(index_vector index) const override; /// Implements set_elem for the view. - void set_elem_(index_vector index, element_type value); + void set_elem_(index_vector index, element_type value) override; /// Implements slice for the view. slice_type slice_(index_vector first_elem, index_vector last_elem); diff --git a/cxx/include/tensorwrapper/tensor/tensor_class.hpp b/cxx/include/tensorwrapper/tensor/tensor_class.hpp index a4ec0655..c7e1e114 100644 --- a/cxx/include/tensorwrapper/tensor/tensor_class.hpp +++ b/cxx/include/tensorwrapper/tensor/tensor_class.hpp @@ -18,6 +18,8 @@ #include #include #include +#include +#include namespace tensorwrapper { namespace detail_ { @@ -85,6 +87,18 @@ class Tensor : public detail_::DSLBase, /// Type of a pointer to a read-only buffer using const_buffer_pointer = input_type::const_buffer_pointer; + /// Type of an individual element of the tensor's buffer + using element_type = input_type::buffer_base::element_type; + + /// Type of a read-only reference to an individual element of the + /// tensor's buffer + using const_element_reference = + input_type::buffer_base::const_element_reference; + + /// Type used to index a single element of the tensor's buffer (one + /// offset per mode, in the buffer's physical index space) + using index_vector = input_type::buffer_base::index_vector; + /// Type used to convey rank using rank_type = typename logical_layout_type::size_type; @@ -319,6 +333,92 @@ class Tensor : public detail_::DSLBase, */ rank_type rank() const; + /** @brief Retrieves an individual element of the tensor's buffer. + * + * @note This operates in the buffer's (physical) index space, not the + * tensor's logical index space (see buffer() for context on the + * distinction). + * + * @param[in] index One offset per mode of the buffer. The length of + * @p index must equal the buffer's rank. + * + * @return The requested element. + * + * @throw std::runtime_error if *this is an empty tensor. Strong throw + * guarantee. + * @throw std::out_of_range if @p index has the wrong length or contains + * an out-of-bounds offset. Strong throw + * guarantee. + */ + const_element_reference get_element(index_vector index) const; + + /** @brief Retrieves an individual element of the tensor's buffer. + * + * This overload allows the offsets to be provided as an arbitrary + * number of arguments (one per mode) instead of as an index_vector. It + * is implemented in terms of get_element(index_vector) const. + * + * @tparam Offsets The types of the offsets. + * + * @param[in] offsets The offsets into each mode of the buffer. + * + * @return The requested element. + * + * @throw std::runtime_error if *this is an empty tensor. Strong throw + * guarantee. + * @throw std::out_of_range if the number of offsets does not equal the + * buffer's rank or if any offset is out of + * bounds. Strong throw guarantee. + */ + template + const_element_reference get_element(Offsets... offsets) const { + return get_element(index_vector{ + static_cast(offsets)...}); + } + + /** @brief Sets an individual element of the tensor's buffer. + * + * @note This operates in the buffer's (physical) index space, not the + * tensor's logical index space (see buffer() for context on the + * distinction). + * + * @param[in] index One offset per mode of the buffer. The length of + * @p index must equal the buffer's rank. + * @param[in] value The new value for the specified element. + * + * @throw std::runtime_error if *this is an empty tensor. Strong throw + * guarantee. + * @throw std::out_of_range if @p index has the wrong length or contains + * an out-of-bounds offset. Strong throw + * guarantee. + */ + void set_element(index_vector index, element_type value); + + /** @brief Sets an individual element of the tensor's buffer. + * + * This overload allows the offsets to be provided as an arbitrary + * number of arguments (one per mode) instead of as an index_vector. It + * is implemented in terms of set_element(index_vector, element_type). + * + * @tparam Offsets The types of the offsets. + * + * @param[in] offsets The offsets into each mode of the buffer. + * @param[in] value The new value for the specified element. + * + * @throw std::runtime_error if *this is an empty tensor. Strong throw + * guarantee. + * @throw std::out_of_range if the number of offsets does not equal the + * buffer's rank or if any offset is out of + * bounds. Strong throw guarantee. + */ + template + void set_element(Args... args) { + static_assert(sizeof...(Args) >= 1, + "set_element requires at least a value argument"); + set_element_unpack_(std::make_index_sequence{}, + std::make_tuple(args...)); + } + // ------------------------------------------------------------------------- // -- Utility methods // ------------------------------------------------------------------------- @@ -417,6 +517,18 @@ class Tensor : public detail_::DSLBase, /// Throws if *this does not have a PIMPL. void assert_pimpl_() const; + /// Splits the last element of @p values off as the new value and + /// forwards the rest as the offsets for set_element(index_vector, + /// element_type). Used to implement the variadic overload of + /// set_element, since a function parameter pack can not be followed by + /// a deduced fixed parameter. + template + void set_element_unpack_(std::index_sequence, Tuple&& values) { + set_element(index_vector{static_cast( + std::get(values))...}, + static_cast(std::get(values))); + } + /// Object actually implementing *this pimpl_pointer m_pimpl_; }; diff --git a/cxx/include/tensorwrapper/types/buffer_traits.hpp b/cxx/include/tensorwrapper/types/buffer_traits.hpp index b044b1db..58170c06 100644 --- a/cxx/include/tensorwrapper/types/buffer_traits.hpp +++ b/cxx/include/tensorwrapper/types/buffer_traits.hpp @@ -34,6 +34,10 @@ struct BufferBaseTraitsCommon : public CommonTypes { using buffer_base_type = buffer::BufferBase; using const_buffer_base_pointer = std::unique_ptr; using const_buffer_base_reference = const buffer_base_type&; + + using element_type = wtf::fp::Float; + using const_element_reference = wtf::fp::FloatView; + using index_vector = std::vector; }; template<> @@ -42,6 +46,7 @@ struct ClassTraits : public BufferBaseTraitsCommon { using layout_pointer = layout_type*; using buffer_base_reference = buffer_base_type&; using buffer_base_pointer = std::unique_ptr; + using element_reference = wtf::fp::FloatView; }; template<> @@ -51,6 +56,7 @@ struct ClassTraits : public BufferBaseTraitsCommon { using buffer_base_reference = const buffer_base_type&; using buffer_base_pointer = std::unique_ptr; using const_buffer_base_pointer = std::unique_ptr; + using element_reference = wtf::fp::FloatView; }; template @@ -69,12 +75,9 @@ struct ClassTraits> : public ClassTraits {}; struct ReplicatedTraitsCommon { - using element_type = wtf::fp::Float; - using const_element_reference = wtf::fp::FloatView; - using buffer_type = wtf::buffer::FloatBuffer; - using const_buffer_view = wtf::buffer::BufferView; - using index_vector = std::vector; - using const_slice_type = buffer::ReplicatedView; + using buffer_type = wtf::buffer::FloatBuffer; + using const_buffer_view = wtf::buffer::BufferView; + using const_slice_type = buffer::ReplicatedView; using slice_il_type = std::initializer_list; }; diff --git a/cxx/src/tensorwrapper/tensor/tensor_class.cpp b/cxx/src/tensorwrapper/tensor/tensor_class.cpp index 9b3f9e55..3e83f4a1 100644 --- a/cxx/src/tensorwrapper/tensor/tensor_class.cpp +++ b/cxx/src/tensorwrapper/tensor/tensor_class.cpp @@ -79,6 +79,16 @@ const_buffer_reference Tensor::buffer() const { Tensor::rank_type Tensor::rank() const { return logical_layout().rank(); } +Tensor::const_element_reference Tensor::get_element(index_vector index) const { + assert_pimpl_(); + return buffer().get_element(std::move(index)); +} + +void Tensor::set_element(index_vector index, element_type value) { + assert_pimpl_(); + buffer().set_element(std::move(index), std::move(value)); +} + // -- Utility void Tensor::swap(Tensor& other) noexcept { m_pimpl_.swap(other.m_pimpl_); } diff --git a/tests/cxx/unit_tests/tensorwrapper/buffer/buffer_base_common.cpp b/tests/cxx/unit_tests/tensorwrapper/buffer/buffer_base_common.cpp index 193508ca..c2ac02ce 100644 --- a/tests/cxx/unit_tests/tensorwrapper/buffer/buffer_base_common.cpp +++ b/tests/cxx/unit_tests/tensorwrapper/buffer/buffer_base_common.cpp @@ -23,6 +23,7 @@ using namespace tensorwrapper; using namespace buffer; +using testing::TestView; TEST_CASE("BufferBaseCommon") { using MutableView = BufferViewBase; @@ -41,12 +42,22 @@ TEST_CASE("BufferBaseCommon") { auto vector_layout = testing::vector_physical(2); buffer::Contiguous defaulted; - MutableView defaulted_view(defaulted); - MutableView scalar_view(scalar); - MutableView vector_view(vector); - ConstView defaulted_const_view(defaulted); - ConstView scalar_const_view(scalar); - ConstView vector_const_view(vector); + + // Concrete objects (BufferViewBase itself is abstract); the rest of this + // test only ever interacts with them through BufferViewBase references. + TestView defaulted_view_impl(defaulted); + TestView scalar_view_impl(scalar); + TestView vector_view_impl(vector); + TestView defaulted_const_view_impl(defaulted); + TestView scalar_const_view_impl(scalar); + TestView vector_const_view_impl(vector); + + MutableView& defaulted_view = defaulted_view_impl; + MutableView& scalar_view = scalar_view_impl; + MutableView& vector_view = vector_view_impl; + ConstView& defaulted_const_view = defaulted_const_view_impl; + ConstView& scalar_const_view = scalar_const_view_impl; + ConstView& vector_const_view = vector_const_view_impl; SECTION("operator== (BufferBase with BufferBaseView)") { REQUIRE(defaulted_view == defaulted); @@ -107,8 +118,46 @@ TEST_CASE("BufferBaseCommon") { } SECTION("Null view equals buffer with no layout") { - ConstView null_view; + TestView null_view_impl; + ConstView& null_view = null_view_impl; REQUIRE(null_view == defaulted); REQUIRE_FALSE(null_view == scalar); } + + SECTION("get_element/set_element (index_vector overload)") { + REQUIRE(scalar.get_element({}) == scalar.get_elem({})); + REQUIRE(vector.get_element({0}) == vector.get_elem({0})); + REQUIRE(vector.get_element({1}) == vector.get_elem({1})); + + scalar.set_element({}, 2.0); + REQUIRE(scalar.get_elem({}) == 2.0); + + vector.set_element({0}, 9.0); + REQUIRE(vector.get_elem({0}) == 9.0); + } + + SECTION("get_element/set_element (variadic offset overload)") { + REQUIRE(scalar.get_element() == scalar.get_elem({})); + REQUIRE(vector.get_element(0) == vector.get_elem({0})); + REQUIRE(vector.get_element(1) == vector.get_elem({1})); + + scalar.set_element(3.0); + REQUIRE(scalar.get_elem({}) == 3.0); + + vector.set_element(0, 8.0); + REQUIRE(vector.get_elem({0}) == 8.0); + + // Wrong number of offsets for the rank still throws, proving the + // variadic overload delegates to (and does not bypass) the existing + // rank/bounds check. + REQUIRE_THROWS_AS(vector.get_element(0, 1), std::out_of_range); + REQUIRE_THROWS_AS(vector.set_element(0, 1, 1.0), std::out_of_range); + } + + SECTION("get_element/set_element on an unsupported view throws") { + REQUIRE_THROWS_AS(scalar_view.get_element({}), std::runtime_error); + REQUIRE_THROWS_AS(scalar_view.set_element({}, 1.0), std::runtime_error); + REQUIRE_THROWS_AS(scalar_const_view.get_element({}), + std::runtime_error); + } } diff --git a/tests/cxx/unit_tests/tensorwrapper/buffer/buffer_view_base.cpp b/tests/cxx/unit_tests/tensorwrapper/buffer/buffer_view_base.cpp index 61a9d9cf..f2fb09e5 100644 --- a/tests/cxx/unit_tests/tensorwrapper/buffer/buffer_view_base.cpp +++ b/tests/cxx/unit_tests/tensorwrapper/buffer/buffer_view_base.cpp @@ -22,6 +22,7 @@ using namespace tensorwrapper; using namespace buffer; +using testing::TestView; TEST_CASE("BufferViewBase") { using MutableView = BufferViewBase; @@ -42,8 +43,10 @@ TEST_CASE("BufferViewBase") { buffer::Contiguous defaulted; SECTION("Default construction") { - ConstView defaulted_const_view; - MutableView defaulted_view; + TestView defaulted_const_view_impl; + TestView defaulted_view_impl; + ConstView& defaulted_const_view = defaulted_const_view_impl; + MutableView& defaulted_view = defaulted_view_impl; REQUIRE_FALSE(defaulted_const_view.has_layout()); REQUIRE_FALSE(defaulted_view.has_layout()); REQUIRE_THROWS_AS(defaulted_const_view.layout(), std::runtime_error); @@ -53,8 +56,10 @@ TEST_CASE("BufferViewBase") { } SECTION("Construct from buffer") { - ConstView scalar_const_view(scalar); - MutableView scalar_view(scalar); + TestView scalar_const_view_impl(scalar); + TestView scalar_view_impl(scalar); + ConstView& scalar_const_view = scalar_const_view_impl; + MutableView& scalar_view = scalar_view_impl; REQUIRE(scalar_const_view.has_layout()); REQUIRE(scalar_view.has_layout()); REQUIRE(scalar_const_view.layout().are_equal(scalar_layout)); @@ -62,8 +67,10 @@ TEST_CASE("BufferViewBase") { REQUIRE(scalar_view.layout().are_equal(scalar_layout)); REQUIRE(scalar_view.rank() == 0); - ConstView vector_const_view(vector); - MutableView vector_view(vector); + TestView vector_const_view_impl(vector); + TestView vector_view_impl(vector); + ConstView& vector_const_view = vector_const_view_impl; + MutableView& vector_view = vector_view_impl; REQUIRE(vector_const_view.has_layout()); REQUIRE(vector_view.has_layout()); REQUIRE(vector_const_view.layout().are_equal(vector_layout)); @@ -73,62 +80,66 @@ TEST_CASE("BufferViewBase") { } SECTION("Copy construction") { - ConstView const_view(scalar); - ConstView copy_const(const_view); + TestView const_view(scalar); + TestView copy_const(const_view); REQUIRE(copy_const.has_layout()); REQUIRE(copy_const.layout().are_equal(scalar_layout)); REQUIRE(copy_const.rank() == 0); - MutableView mutable_view(scalar); - MutableView copy_mutable(mutable_view); + TestView mutable_view(scalar); + TestView copy_mutable(mutable_view); REQUIRE(copy_mutable.has_layout()); REQUIRE(copy_mutable.layout().are_equal(scalar_layout)); REQUIRE(copy_mutable.rank() == 0); } SECTION("Move construction") { - ConstView const_view(scalar); - ConstView moved_const(std::move(const_view)); + TestView const_view(scalar); + TestView moved_const(std::move(const_view)); REQUIRE(moved_const.has_layout()); REQUIRE(moved_const.layout().are_equal(scalar_layout)); REQUIRE(moved_const.rank() == 0); - MutableView mutable_view(scalar); - MutableView moved(std::move(mutable_view)); + TestView mutable_view(scalar); + TestView moved(std::move(mutable_view)); REQUIRE(moved.has_layout()); REQUIRE(moved.layout().are_equal(scalar_layout)); REQUIRE(moved.rank() == 0); } SECTION("Copy assignment") { - ConstView const_view(scalar); - ConstView other_const; - auto pother_const = &(other_const = const_view); + TestView const_view(scalar); + TestView other_const_impl; + ConstView& other_const = other_const_impl; + auto pother_const = &(other_const = const_view); REQUIRE(pother_const == &other_const); REQUIRE(other_const.has_layout()); REQUIRE(other_const.layout().are_equal(scalar_layout)); REQUIRE(other_const.rank() == 0); - MutableView mutable_view(scalar); - MutableView other; - other = mutable_view; + TestView mutable_view(scalar); + TestView other_impl; + MutableView& other = other_impl; + other = mutable_view; REQUIRE(other.has_layout()); REQUIRE(other.layout().are_equal(scalar_layout)); REQUIRE(other.rank() == 0); } SECTION("Move assignment") { - ConstView const_view(scalar); - ConstView other_const; - auto pother_const = &(other_const = std::move(const_view)); + TestView const_view(scalar); + TestView other_const_impl; + ConstView& other_const = other_const_impl; + auto pother_const = &(other_const = std::move(const_view)); REQUIRE(pother_const == &other_const); REQUIRE(other_const.has_layout()); REQUIRE(other_const.layout().are_equal(scalar_layout)); REQUIRE(other_const.rank() == 0); - MutableView mutable_view(scalar); - MutableView other_mutable; - other_mutable = std::move(mutable_view); + TestView mutable_view(scalar); + TestView other_mutable_impl; + MutableView& other_mutable = other_mutable_impl; + other_mutable = std::move(mutable_view); REQUIRE(other_mutable.has_layout()); REQUIRE(other_mutable.layout().are_equal(scalar_layout)); REQUIRE(other_mutable.rank() == 0); diff --git a/tests/cxx/unit_tests/tensorwrapper/buffer/replicated_view.cpp b/tests/cxx/unit_tests/tensorwrapper/buffer/replicated_view.cpp index c2d66233..66c935f2 100644 --- a/tests/cxx/unit_tests/tensorwrapper/buffer/replicated_view.cpp +++ b/tests/cxx/unit_tests/tensorwrapper/buffer/replicated_view.cpp @@ -120,6 +120,28 @@ TEST_CASE("ReplicatedView") { REQUIRE(matrix.get_elem({0, 1}) == nine_nine); } + SECTION("get_element/set_element (BufferBaseCommon path)") { + // Regression test: BufferBaseCommon>::get_element() + // dispatches through the virtual get_elem_()/set_elem_() hooks + // declared on BufferViewBase. This proves that dispatch actually + // reaches ReplicatedView's slice-aware override (via SlicePIMPL's + // index translation) instead of silently falling through to + // BufferViewBase's throwing default implementation. + REQUIRE(vector_view.get_element({0}) == vector_view.get_elem({0})); + REQUIRE(vector_view.get_element({0}) == vector.get_elem({1})); + REQUIRE(vector_view.get_element({1}) == vector.get_elem({2})); + + REQUIRE(const_vector_view.get_element({0}) == + const_vector_view.get_elem({0})); + + vector_view.set_element({0}, 42.0); + REQUIRE(vector.get_elem({1}) == 42.0); + REQUIRE(vector_view.get_element({0}) == 42.0); + + REQUIRE(matrix_view.get_element({0, 0}) == matrix.get_elem({0, 1})); + REQUIRE(matrix_view.get_element({1, 0}) == matrix.get_elem({1, 1})); + } + SECTION("slice() const") { auto vector_slice = std::as_const(vector_view).slice({1}, {2}); REQUIRE(vector_slice.layout().shape().size() == 1); diff --git a/tests/cxx/unit_tests/tensorwrapper/tensor/tensor_class.cpp b/tests/cxx/unit_tests/tensorwrapper/tensor/tensor_class.cpp index f6a1ebf0..ad901c16 100644 --- a/tests/cxx/unit_tests/tensorwrapper/tensor/tensor_class.cpp +++ b/tests/cxx/unit_tests/tensorwrapper/tensor/tensor_class.cpp @@ -128,6 +128,37 @@ TEST_CASE("Tensor") { REQUIRE_THROWS_AS(defaulted.rank(), std::runtime_error); } + SECTION("get_element/set_element (index_vector overload)") { + REQUIRE(scalar.get_element({}) == 42.0); + REQUIRE(vector.get_element({0}) == 0.0); + REQUIRE(vector.get_element({4}) == 4.0); + + scalar.set_element({}, 99.0); + REQUIRE(scalar.get_element({}) == 99.0); + + vector.set_element({0}, 100.0); + REQUIRE(vector.get_element({0}) == 100.0); + + REQUIRE_THROWS_AS(defaulted.get_element({}), std::runtime_error); + REQUIRE_THROWS_AS(defaulted.set_element({}, 1.0), std::runtime_error); + } + + SECTION("get_element/set_element (variadic offset overload)") { + REQUIRE(scalar.get_element() == 42.0); + REQUIRE(vector.get_element(0) == 0.0); + REQUIRE(vector.get_element(4) == 4.0); + + scalar.set_element(99.0); + REQUIRE(scalar.get_element() == 99.0); + + vector.set_element(0, 100.0); + REQUIRE(vector.get_element(0) == 100.0); + + REQUIRE_THROWS_AS(vector.get_element(0, 1), std::out_of_range); + REQUIRE_THROWS_AS(vector.set_element(0, 1, 1.0), std::out_of_range); + REQUIRE_THROWS_AS(defaulted.get_element(), std::runtime_error); + } + SECTION("swap") { Tensor scalar_copy(scalar); Tensor vector_copy(vector); diff --git a/tests/cxx/unit_tests/tensorwrapper/testing/test_view.hpp b/tests/cxx/unit_tests/tensorwrapper/testing/test_view.hpp new file mode 100644 index 00000000..80738051 --- /dev/null +++ b/tests/cxx/unit_tests/tensorwrapper/testing/test_view.hpp @@ -0,0 +1,51 @@ +/* + * Copyright 2024 NWChemEx-Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once +#include +#include + +namespace tensorwrapper::testing { + +/// BufferViewBase is abstract (get_elem_()/set_elem_() are pure virtual), so +/// it can not be instantiated directly. This minimal derived class exists so +/// tests that only need BufferViewBase's own layout/construction/equality +/// behavior (and not real element access) can still get a concrete object. +/// get_elem_()/set_elem_() just throw, mirroring what an unsupported view +/// would do. +template +class TestView : public buffer::BufferViewBase { +private: + using base_type = buffer::BufferViewBase; + +public: + using base_type::base_type; + +protected: + using typename base_type::const_element_reference; + using typename base_type::element_type; + using typename base_type::index_vector; + + const_element_reference get_elem_(index_vector) const override { + throw std::runtime_error("TestView does not implement get_elem_"); + } + + void set_elem_(index_vector, element_type) override { + throw std::runtime_error("TestView does not implement set_elem_"); + } +}; + +} // namespace tensorwrapper::testing diff --git a/tests/cxx/unit_tests/tensorwrapper/testing/testing.hpp b/tests/cxx/unit_tests/tensorwrapper/testing/testing.hpp index 76053fad..f6aa487d 100644 --- a/tests/cxx/unit_tests/tensorwrapper/testing/testing.hpp +++ b/tests/cxx/unit_tests/tensorwrapper/testing/testing.hpp @@ -21,5 +21,6 @@ #include "inputs.hpp" #include "layouts.hpp" #include "shapes.hpp" +#include "test_view.hpp" namespace tensorwrapper::testing {} // namespace tensorwrapper::testing