From 436b36bdf068481500acddb9d44016c0adfe1993 Mon Sep 17 00:00:00 2001 From: odlomax Date: Fri, 23 Aug 2024 00:40:00 +0100 Subject: [PATCH 01/19] Added array view variant class and tests. --- src/atlas/CMakeLists.txt | 1 + src/atlas/util/ArrayViewVariant.h | 167 ++++++++++++++++++++++++ src/tests/util/CMakeLists.txt | 6 + src/tests/util/test_arrayviewvariant.cc | 130 ++++++++++++++++++ 4 files changed, 304 insertions(+) create mode 100644 src/atlas/util/ArrayViewVariant.h create mode 100644 src/tests/util/test_arrayviewvariant.cc diff --git a/src/atlas/CMakeLists.txt b/src/atlas/CMakeLists.txt index d3f0c6b5c..290648937 100644 --- a/src/atlas/CMakeLists.txt +++ b/src/atlas/CMakeLists.txt @@ -806,6 +806,7 @@ util/PeriodicTransform.h util/Topology.h util/Allocate.h util/Allocate.cc +util/ArrayViewVariant.h util/GPUClonable.h util/Constants.h diff --git a/src/atlas/util/ArrayViewVariant.h b/src/atlas/util/ArrayViewVariant.h new file mode 100644 index 000000000..dd76e199c --- /dev/null +++ b/src/atlas/util/ArrayViewVariant.h @@ -0,0 +1,167 @@ +#pragma once +#pragma once + +#include +#include + +#include "atlas/array.h" +#include "atlas/runtime/Exception.h" + +namespace atlas { +namespace util { + +namespace detail { + +using namespace array; + +// Container struct for a list of types. +template +struct TypeList {}; + +// Container struct for a list of integers. +template +struct IntList {}; + +// Check if type is a TypeList. +template +struct IsTypeList : std::false_type {}; + +template +struct IsTypeList> : std::true_type {}; + +// Check if type is an IntList. +template +struct IsIntList : std::false_type {}; + +template +struct IsIntList> : std::true_type {}; + +// Concatenate two TypeLisTypes. +template +constexpr auto concatenateTypeLisTypes(TypeList, TypeList) { + return TypeList{}; +} + +// Convert types in a TypeList to const types. +template +constexpr auto constTypeListImpl(TypeList) { + return TypeList...>{}; +} + +template +using ConstTypeList = decltype(constTypeListImpl(Types{})); + +// Convert a TypeList to a std::variant of the same types. +template +struct TypeListToVariantImpl { + constexpr TypeListToVariantImpl(TypeList) {}; + using variant = std::variant; +}; + +template +using TypeListToVariant = + typename decltype(TypeListToVariantImpl(Types{}))::variant; + +// Iterate over each rank in an IntList. Create a TypeList of ArrayViews. +template +constexpr auto forEachRank(TypeList, IntList) { + return concatenateTypeLisTypes( + TypeList>{}, + forEachRank(TypeList{}, IntList{})); +} + +template +constexpr auto forEachRank(TypeList, IntList<>) { + return TypeList<>{}; +} + +// Iterate over each value-type in a TypeList and each rank in an IntList. +// Create a TypeList of ArrayViews. +template +constexpr auto forEachValue(TypeList, IntList) { + return concatenateTypeLisTypes( + forEachRank(TypeList{}, IntList{}), + forEachValue(TypeList{}, IntList{})); +} + +template +constexpr auto forEachValue(TypeList<>, IntList) { + return TypeList<>{}; +} + +// Get TypeList of ArrayViews. +template +using GetArrayViews = decltype(forEachValue(Values{}, Ranks{})); + +// Match an Array to the ArrayView types in variant. Once found, return +// ArrayView. Throw exception if Array is not matched. +template +Variant setVariant(Arr& arr, TypeList) { + constexpr auto Rank = View::rank(); + using Value = typename View::value_type; + + if (arr.rank() == Rank && array::DataType::kind() == arr.datatype()) { + return Variant{array::make_view(arr)}; + } + return setVariant(arr, TypeList{}); +} + +template +Variant setVariant(Arr& arr, TypeList<>) { + const auto datatypeStr = array::DataType::kind_to_str(arr.datatype().kind()); + const auto rankStr = std::to_string(arr.rank()); + throw_Exception("Could not find ArrayView<" + datatypeStr + ", " + rankStr + + "> in variant type.", + Here()); +} + +} // namespace detail + +/// @brief Helper struct to contain a list of array value-types. +template +using Values = detail::TypeList; + +/// @brief Helper struct to contain a list of array ranks. +template +using Ranks = detail::IntList; + +/// @brief Default value-types for ArrayView variant. +using DefaultValues = Values; + +/// @brief Default ranks for ArrayView variant. +using DefaultRanks = Ranks<1, 2, 3>; + + +/// @ brief Takes an array and creates a std::variant of possible ArrayViews. +/// +/// @details Possible array views can be set by providing Values and +/// Ranks as template argumenTypes. +template ::value>, + typename = std::enable_if_t::value>> +auto make_array_view_variant(array::Array& array) { + using ArrayViewList = detail::GetArrayViews; + using Variant = detail::TypeListToVariant; + return detail::setVariant(array, ArrayViewList{}); +} + +/// @ brief Takes a const array and creates a std::variant of possible +/// ArrayViews. +/// +/// @details Possible array views can be set by providing Values and +/// Ranks as template arguments. Value-types are converted to +/// const before constructing the ArrayView variant. +template ::value>, + typename = std::enable_if_t::value>> +auto make_array_view_variant(const array::Array& array) { + using ConstValuesList = detail::ConstTypeList; + using ArrayViewList = detail::GetArrayViews; + using Variant = detail::TypeListToVariant; + return detail::setVariant(array, ArrayViewList{}); +} + +} // namespace util +} // namespace atlas diff --git a/src/tests/util/CMakeLists.txt b/src/tests/util/CMakeLists.txt index 47d8b6def..aa5c78181 100644 --- a/src/tests/util/CMakeLists.txt +++ b/src/tests/util/CMakeLists.txt @@ -94,3 +94,9 @@ ecbuild_add_test( TARGET atlas_test_unitsphere ENVIRONMENT ${ATLAS_TEST_ENVIRONMENT} ) +ecbuild_add_test( TARGET atlas_test_arrayviewviewvariant + SOURCES test_arrayviewvariant.cc + LIBS atlas + ENVIRONMENT ${ATLAS_TEST_ENVIRONMENT} +) + diff --git a/src/tests/util/test_arrayviewvariant.cc b/src/tests/util/test_arrayviewvariant.cc new file mode 100644 index 000000000..4afff30eb --- /dev/null +++ b/src/tests/util/test_arrayviewvariant.cc @@ -0,0 +1,130 @@ +/* + * (C) Crown Copyright 2024 Met Office + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + */ + +#include +#include +#include + +#include "atlas/array.h" +#include "atlas/util/ArrayViewVariant.h" +#include "tests/AtlasTestEnvironment.h" + +namespace atlas { +namespace test { + +using namespace array; +using namespace util; + +CASE("test visit") { + auto arr1 = array::ArrayT(2); + auto arr2 = array::ArrayT(2, 3); + auto arr3 = array::ArrayT(2, 3, 4); + + using ValueList = Values; + using RankList = Ranks<1, 2, 3>; + + const auto var1 = make_array_view_variant(arr1); + const auto var2 = make_array_view_variant(arr2); + const auto var3 = make_array_view_variant(arr3); + + std::visit( + [](auto&& view) { + using View = std::remove_reference_t; + EXPECT_EQ(View::rank(), 1); + EXPECT((std::is_same_v)); + }, + var1); + + std::visit( + [](auto&& view) { + using View = std::remove_reference_t; + EXPECT_EQ(View::rank(), 2); + EXPECT((std::is_same_v)); + }, + var2); + + std::visit( + [](auto&& view) { + using View = std::remove_reference_t; + EXPECT_EQ(View::rank(), 3); + EXPECT((std::is_same_v)); + }, + var3); +} + +CASE("test array view data") { + auto arr = ArrayT(10); + make_view(arr).assign({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); + + const auto& arrRef = arr; + const auto var = + make_array_view_variant, Ranks<1, 2>>(arrRef); + + std::visit( + [](auto&& view) { + using View = std::remove_reference_t; + EXPECT_EQ(View::rank(), 1); + if constexpr (View::rank() == 1) { + for (auto i = idx_t{0}; i < view.size(); ++i) { + EXPECT_EQ(view(i), i); + } + } + }, + var); +} + +CASE("test instantiation") { + auto arr = array::ArrayT(1); + const auto constArr = array::ArrayT(1); + + SECTION("default variants") { + auto var = make_array_view_variant(arr); + auto constVar = make_array_view_variant(constArr); + + using VarType = std::variant, ArrayView, + ArrayView, ArrayView, + ArrayView, ArrayView>; + + using ConstVarType = + std::variant, ArrayView, + ArrayView, ArrayView, + ArrayView, ArrayView>; + + EXPECT((std::is_same_v)); + EXPECT((std::is_same_v)); + } + + SECTION("customised variants") { + using ValueList = Values; + using RankList = Ranks<1>; + + auto var = make_array_view_variant(arr); + auto constVar = make_array_view_variant(constArr); + + using VarType = std::variant, ArrayView>; + + using ConstVarType = + std::variant, ArrayView>; + + EXPECT((std::is_same_v)); + EXPECT((std::is_same_v)); + } + + SECTION("mismatched array and variant") { + // Array is of value-type double. + using ValueList = Values; + using RankList = Ranks<1>; + + EXPECT_THROWS((make_array_view_variant(arr))); + EXPECT_THROWS((make_array_view_variant(constArr))); + } +} + +} // namespace test +} // namespace atlas + +int main(int argc, char** argv) { return atlas::test::run(argc, argv); } From 30a2bc3c8ef29cf1b0247fa7e4e59012260464a3 Mon Sep 17 00:00:00 2001 From: odlomax Date: Fri, 6 Sep 2024 09:51:29 +0100 Subject: [PATCH 02/19] Moved make_view_variant into array namespace. --- src/atlas/CMakeLists.txt | 2 +- src/atlas/{util => array}/ArrayViewVariant.h | 6 ++--- src/tests/array/CMakeLists.txt | 5 ++++ .../{util => array}/test_arrayviewvariant.cc | 23 +++++++++---------- src/tests/util/CMakeLists.txt | 5 ---- 5 files changed, 20 insertions(+), 21 deletions(-) rename src/atlas/{util => array}/ArrayViewVariant.h (97%) rename src/tests/{util => array}/test_arrayviewvariant.cc (81%) diff --git a/src/atlas/CMakeLists.txt b/src/atlas/CMakeLists.txt index 9ef761606..782b4ff2e 100644 --- a/src/atlas/CMakeLists.txt +++ b/src/atlas/CMakeLists.txt @@ -741,6 +741,7 @@ array/Range.h array/Vector.h array/Vector.cc array/SVector.h +array/ArrayViewVariant.h array/helpers/ArrayInitializer.h array/helpers/ArrayAssigner.h array/helpers/ArrayWriter.h @@ -806,7 +807,6 @@ util/PeriodicTransform.h util/Topology.h util/Allocate.h util/Allocate.cc -util/ArrayViewVariant.h util/GPUClonable.h util/Constants.h diff --git a/src/atlas/util/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h similarity index 97% rename from src/atlas/util/ArrayViewVariant.h rename to src/atlas/array/ArrayViewVariant.h index dd76e199c..c1a6cb228 100644 --- a/src/atlas/util/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -8,7 +8,7 @@ #include "atlas/runtime/Exception.h" namespace atlas { -namespace util { +namespace array { namespace detail { @@ -140,7 +140,7 @@ template ::value>, typename = std::enable_if_t::value>> -auto make_array_view_variant(array::Array& array) { +auto make_view_variant(array::Array& array) { using ArrayViewList = detail::GetArrayViews; using Variant = detail::TypeListToVariant; return detail::setVariant(array, ArrayViewList{}); @@ -156,7 +156,7 @@ template ::value>, typename = std::enable_if_t::value>> -auto make_array_view_variant(const array::Array& array) { +auto make_view_variant(const array::Array& array) { using ConstValuesList = detail::ConstTypeList; using ArrayViewList = detail::GetArrayViews; using Variant = detail::TypeListToVariant; diff --git a/src/tests/array/CMakeLists.txt b/src/tests/array/CMakeLists.txt index 3915caca7..eda04a6d1 100644 --- a/src/tests/array/CMakeLists.txt +++ b/src/tests/array/CMakeLists.txt @@ -81,3 +81,8 @@ atlas_add_hic_test( ENVIRONMENT ${ATLAS_TEST_ENVIRONMENT} ) +ecbuild_add_test( TARGET atlas_test_arrayviewviewvariant + SOURCES test_arrayviewvariant.cc + LIBS atlas + ENVIRONMENT ${ATLAS_TEST_ENVIRONMENT} +) diff --git a/src/tests/util/test_arrayviewvariant.cc b/src/tests/array/test_arrayviewvariant.cc similarity index 81% rename from src/tests/util/test_arrayviewvariant.cc rename to src/tests/array/test_arrayviewvariant.cc index 4afff30eb..2b143f9aa 100644 --- a/src/tests/util/test_arrayviewvariant.cc +++ b/src/tests/array/test_arrayviewvariant.cc @@ -9,15 +9,14 @@ #include #include +#include "atlas/array/ArrayViewVariant.h" #include "atlas/array.h" -#include "atlas/util/ArrayViewVariant.h" #include "tests/AtlasTestEnvironment.h" namespace atlas { namespace test { using namespace array; -using namespace util; CASE("test visit") { auto arr1 = array::ArrayT(2); @@ -27,9 +26,9 @@ CASE("test visit") { using ValueList = Values; using RankList = Ranks<1, 2, 3>; - const auto var1 = make_array_view_variant(arr1); - const auto var2 = make_array_view_variant(arr2); - const auto var3 = make_array_view_variant(arr3); + const auto var1 = make_view_variant(arr1); + const auto var2 = make_view_variant(arr2); + const auto var3 = make_view_variant(arr3); std::visit( [](auto&& view) { @@ -62,7 +61,7 @@ CASE("test array view data") { const auto& arrRef = arr; const auto var = - make_array_view_variant, Ranks<1, 2>>(arrRef); + make_view_variant, Ranks<1, 2>>(arrRef); std::visit( [](auto&& view) { @@ -82,8 +81,8 @@ CASE("test instantiation") { const auto constArr = array::ArrayT(1); SECTION("default variants") { - auto var = make_array_view_variant(arr); - auto constVar = make_array_view_variant(constArr); + auto var = make_view_variant(arr); + auto constVar = make_view_variant(constArr); using VarType = std::variant, ArrayView, ArrayView, ArrayView, @@ -102,8 +101,8 @@ CASE("test instantiation") { using ValueList = Values; using RankList = Ranks<1>; - auto var = make_array_view_variant(arr); - auto constVar = make_array_view_variant(constArr); + auto var = make_view_variant(arr); + auto constVar = make_view_variant(constArr); using VarType = std::variant, ArrayView>; @@ -119,8 +118,8 @@ CASE("test instantiation") { using ValueList = Values; using RankList = Ranks<1>; - EXPECT_THROWS((make_array_view_variant(arr))); - EXPECT_THROWS((make_array_view_variant(constArr))); + EXPECT_THROWS((make_view_variant(arr))); + EXPECT_THROWS((make_view_variant(constArr))); } } diff --git a/src/tests/util/CMakeLists.txt b/src/tests/util/CMakeLists.txt index aa5c78181..703d310fd 100644 --- a/src/tests/util/CMakeLists.txt +++ b/src/tests/util/CMakeLists.txt @@ -94,9 +94,4 @@ ecbuild_add_test( TARGET atlas_test_unitsphere ENVIRONMENT ${ATLAS_TEST_ENVIRONMENT} ) -ecbuild_add_test( TARGET atlas_test_arrayviewviewvariant - SOURCES test_arrayviewvariant.cc - LIBS atlas - ENVIRONMENT ${ATLAS_TEST_ENVIRONMENT} -) From 95d997750b105b0e219b0c6ca731a96f997f60b7 Mon Sep 17 00:00:00 2001 From: odlomax Date: Wed, 11 Sep 2024 09:26:00 +0100 Subject: [PATCH 03/19] Refactored ArrayViewVariant methods. --- src/atlas/CMakeLists.txt | 1 + src/atlas/array/ArrayViewVariant.cc | 101 ++++++++++++ src/atlas/array/ArrayViewVariant.h | 192 +++++++---------------- src/tests/array/test_arrayviewvariant.cc | 91 +++++------ 4 files changed, 194 insertions(+), 191 deletions(-) create mode 100644 src/atlas/array/ArrayViewVariant.cc diff --git a/src/atlas/CMakeLists.txt b/src/atlas/CMakeLists.txt index 782b4ff2e..e3efe0053 100644 --- a/src/atlas/CMakeLists.txt +++ b/src/atlas/CMakeLists.txt @@ -742,6 +742,7 @@ array/Vector.h array/Vector.cc array/SVector.h array/ArrayViewVariant.h +array/ArrayViewVariant.cc array/helpers/ArrayInitializer.h array/helpers/ArrayAssigner.h array/helpers/ArrayWriter.h diff --git a/src/atlas/array/ArrayViewVariant.cc b/src/atlas/array/ArrayViewVariant.cc new file mode 100644 index 000000000..af59fd3d2 --- /dev/null +++ b/src/atlas/array/ArrayViewVariant.cc @@ -0,0 +1,101 @@ +/* + * (C) Crown Copyright 2024 Met Office + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + */ + +#include "atlas/array/ArrayViewVariant.h" + +#include + +#include "atlas/runtime/Exception.h" + +namespace atlas { +namespace array { + +using namespace detail; + +namespace { + +// Match array.rank() and array.datatype() to variant types. Return result of +// makeView on a successfull pattern match. +template +struct MakeViewHelper { + template + static ArrayViewVariant apply(ArrayType& array, const MakeView& makeView) { + using View = std::variant_alternative_t; + using Value = typename View::value_type; + constexpr auto Rank = View::rank(); + + if (array.datatype() == DataType::kind() && array.rank() == Rank) { + return makeView(array, Value{}, std::integral_constant{}); + } + return MakeViewHelper::apply(array, makeView); + } +}; + +// End recursion. +template <> +struct MakeViewHelper> { + template + static ArrayViewVariant apply(ArrayType& array, const MakeView& makeView) { + throw_Exception("ArrayView<" + array.datatype().str() + ", " + + std::to_string(array.rank()) + + "> is not an alternative in ArrayViewVariant.", + Here()); + } +}; + +template +ArrayViewVariant makeViewVariantImpl(ArrayType& array) { + const auto makeView = [](auto& array, auto value, auto rank) { + return make_view(array); + }; + return MakeViewHelper<0>::apply(array, makeView); +} + +template +ArrayViewVariant makeHostViewVariantImpl(ArrayType& array) { + const auto makeView = [](auto& array, auto value, auto rank) { + return make_host_view(array); + }; + return MakeViewHelper<0>::apply(array, makeView); +} + +template +ArrayViewVariant makeDeviceViewVariantImpl(ArrayType& array) { + const auto makeView = [](auto& array, auto value, auto rank) { + return make_device_view(array); + }; + return MakeViewHelper<0>::apply(array, makeView); +} + +} // namespace + +ArrayViewVariant make_view_variant(Array& array) { + return makeViewVariantImpl(array); +} + +ArrayViewVariant make_view_variant(const Array& array) { + return makeViewVariantImpl(array); +} + +ArrayViewVariant make_host_view_variant(Array& array) { + return makeHostViewVariantImpl(array); +} + +ArrayViewVariant make_host_view_variant(const Array& array) { + return makeHostViewVariantImpl(array); +} + +ArrayViewVariant make_devive_view_variant(Array& array) { + return makeDeviceViewVariantImpl(array); +} + +ArrayViewVariant make_device_view_variant(const Array& array) { + return makeDeviceViewVariantImpl(array); +} + +} // namespace array +} // namespace atlas diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index c1a6cb228..5fc2f9270 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -1,11 +1,16 @@ -#pragma once +/* + * (C) Crown Copyright 2024 Met Office + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + */ + #pragma once -#include #include #include "atlas/array.h" -#include "atlas/runtime/Exception.h" +#include "eckit/utils/Overloaded.h" namespace atlas { namespace array { @@ -19,149 +24,64 @@ template struct TypeList {}; // Container struct for a list of integers. -template +template struct IntList {}; -// Check if type is a TypeList. -template -struct IsTypeList : std::false_type {}; +// Supported ArrayView value types. +constexpr auto ValueList = TypeList{}; + +// Supported ArrayView ranks. +constexpr auto RankList = IntList<1, 2, 3, 4, 5, 6, 7, 8, 9>{}; + +// Helper struct to build an ArrayView variant from a list of value types and +// and a list of ranks. +template +struct VariantBuilder { + using type = std::variant; + + // Make a VariantBuilder struct with a fully populated Views... argument. + template + static constexpr auto make(TypeList, IntList) { + return VariantBuilder< + Views..., ArrayView..., + ArrayView...>::make(TypeList{}, + IntList{}); + } -template -struct IsTypeList> : std::true_type {}; + // End recursion. + template + static constexpr VariantBuilder make(TypeList<>, + IntList) { + return VariantBuilder{}; + } +}; + +} // namespace detail -// Check if type is an IntList. -template -struct IsIntList : std::false_type {}; +/// @brief Variant containing all supported ArrayView alternatives. +using ArrayViewVariant = decltype(detail::VariantBuilder<>::make( + detail::ValueList, detail::RankList))::type; -template -struct IsIntList> : std::true_type {}; +/// @brief Use overloaded pattern as visitor. +using eckit::Overloaded; -// Concatenate two TypeLisTypes. -template -constexpr auto concatenateTypeLisTypes(TypeList, TypeList) { - return TypeList{}; -} +/// @brief Create an ArrayView and assign to an ArrayViewVariant. +ArrayViewVariant make_view_variant(array::Array& array); -// Convert types in a TypeList to const types. -template -constexpr auto constTypeListImpl(TypeList) { - return TypeList...>{}; -} +/// @brief Create a const ArrayView and assign to an ArrayViewVariant. +ArrayViewVariant make_view_variant(const array::Array& array); -template -using ConstTypeList = decltype(constTypeListImpl(Types{})); +/// @brief Create a host ArrayView and assign to an ArrayViewVariant. +ArrayViewVariant make_host_view_variant(array::Array& array); -// Convert a TypeList to a std::variant of the same types. -template -struct TypeListToVariantImpl { - constexpr TypeListToVariantImpl(TypeList) {}; - using variant = std::variant; -}; +/// @brief Create a host const ArrayView and assign to an ArrayViewVariant. +ArrayViewVariant make_host_view_variant(const array::Array& array); -template -using TypeListToVariant = - typename decltype(TypeListToVariantImpl(Types{}))::variant; - -// Iterate over each rank in an IntList. Create a TypeList of ArrayViews. -template -constexpr auto forEachRank(TypeList, IntList) { - return concatenateTypeLisTypes( - TypeList>{}, - forEachRank(TypeList{}, IntList{})); -} - -template -constexpr auto forEachRank(TypeList, IntList<>) { - return TypeList<>{}; -} - -// Iterate over each value-type in a TypeList and each rank in an IntList. -// Create a TypeList of ArrayViews. -template -constexpr auto forEachValue(TypeList, IntList) { - return concatenateTypeLisTypes( - forEachRank(TypeList{}, IntList{}), - forEachValue(TypeList{}, IntList{})); -} - -template -constexpr auto forEachValue(TypeList<>, IntList) { - return TypeList<>{}; -} - -// Get TypeList of ArrayViews. -template -using GetArrayViews = decltype(forEachValue(Values{}, Ranks{})); - -// Match an Array to the ArrayView types in variant. Once found, return -// ArrayView. Throw exception if Array is not matched. -template -Variant setVariant(Arr& arr, TypeList) { - constexpr auto Rank = View::rank(); - using Value = typename View::value_type; - - if (arr.rank() == Rank && array::DataType::kind() == arr.datatype()) { - return Variant{array::make_view(arr)}; - } - return setVariant(arr, TypeList{}); -} - -template -Variant setVariant(Arr& arr, TypeList<>) { - const auto datatypeStr = array::DataType::kind_to_str(arr.datatype().kind()); - const auto rankStr = std::to_string(arr.rank()); - throw_Exception("Could not find ArrayView<" + datatypeStr + ", " + rankStr + - "> in variant type.", - Here()); -} +/// @brief Create a device ArrayView and assign to an ArrayViewVariant. +ArrayViewVariant make_devive_view_variant(array::Array& array); -} // namespace detail +/// @brief Create a const device ArrayView and assign to an ArrayViewVariant. +ArrayViewVariant make_device_view_variant(const array::Array& array); -/// @brief Helper struct to contain a list of array value-types. -template -using Values = detail::TypeList; - -/// @brief Helper struct to contain a list of array ranks. -template -using Ranks = detail::IntList; - -/// @brief Default value-types for ArrayView variant. -using DefaultValues = Values; - -/// @brief Default ranks for ArrayView variant. -using DefaultRanks = Ranks<1, 2, 3>; - - -/// @ brief Takes an array and creates a std::variant of possible ArrayViews. -/// -/// @details Possible array views can be set by providing Values and -/// Ranks as template argumenTypes. -template ::value>, - typename = std::enable_if_t::value>> -auto make_view_variant(array::Array& array) { - using ArrayViewList = detail::GetArrayViews; - using Variant = detail::TypeListToVariant; - return detail::setVariant(array, ArrayViewList{}); -} - -/// @ brief Takes a const array and creates a std::variant of possible -/// ArrayViews. -/// -/// @details Possible array views can be set by providing Values and -/// Ranks as template arguments. Value-types are converted to -/// const before constructing the ArrayView variant. -template ::value>, - typename = std::enable_if_t::value>> -auto make_view_variant(const array::Array& array) { - using ConstValuesList = detail::ConstTypeList; - using ArrayViewList = detail::GetArrayViews; - using Variant = detail::TypeListToVariant; - return detail::setVariant(array, ArrayViewList{}); -} - -} // namespace util +} // namespace array } // namespace atlas diff --git a/src/tests/array/test_arrayviewvariant.cc b/src/tests/array/test_arrayviewvariant.cc index 2b143f9aa..2ab24bd2c 100644 --- a/src/tests/array/test_arrayviewvariant.cc +++ b/src/tests/array/test_arrayviewvariant.cc @@ -9,8 +9,8 @@ #include #include -#include "atlas/array/ArrayViewVariant.h" #include "atlas/array.h" +#include "atlas/array/ArrayViewVariant.h" #include "tests/AtlasTestEnvironment.h" namespace atlas { @@ -23,12 +23,9 @@ CASE("test visit") { auto arr2 = array::ArrayT(2, 3); auto arr3 = array::ArrayT(2, 3, 4); - using ValueList = Values; - using RankList = Ranks<1, 2, 3>; - - const auto var1 = make_view_variant(arr1); - const auto var2 = make_view_variant(arr2); - const auto var3 = make_view_variant(arr3); + const auto var1 = make_view_variant(arr1); + const auto var2 = make_view_variant(arr2); + const auto var3 = make_view_variant(arr3); std::visit( [](auto&& view) { @@ -55,72 +52,56 @@ CASE("test visit") { var3); } +template +constexpr auto Rank = std::decay_t::rank(); + CASE("test array view data") { auto arr = ArrayT(10); make_view(arr).assign({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); const auto& arrRef = arr; - const auto var = - make_view_variant, Ranks<1, 2>>(arrRef); + const auto var = make_view_variant(arrRef); - std::visit( - [](auto&& view) { - using View = std::remove_reference_t; + const auto visitor = Overloaded{ + [](auto&& view) -> std::enable_if_t == 1> { + using View = std::decay_t; EXPECT_EQ(View::rank(), 1); - if constexpr (View::rank() == 1) { - for (auto i = idx_t{0}; i < view.size(); ++i) { - EXPECT_EQ(view(i), i); - } + using Value = typename View::value_type; + EXPECT((std::is_same_v)); + + for (auto i = size_t{0}; i < view.size(); ++i) { + EXPECT_EQ(view(i), static_cast(i)); } }, - var); + [](auto&& view) -> std::enable_if_t != 1> { + // do nothing. + }}; + + std::visit(visitor, var); } CASE("test instantiation") { auto arr = array::ArrayT(1); const auto constArr = array::ArrayT(1); - SECTION("default variants") { - auto var = make_view_variant(arr); - auto constVar = make_view_variant(constArr); - - using VarType = std::variant, ArrayView, - ArrayView, ArrayView, - ArrayView, ArrayView>; - - using ConstVarType = - std::variant, ArrayView, - ArrayView, ArrayView, - ArrayView, ArrayView>; - - EXPECT((std::is_same_v)); - EXPECT((std::is_same_v)); - } - - SECTION("customised variants") { - using ValueList = Values; - using RankList = Ranks<1>; - - auto var = make_view_variant(arr); - auto constVar = make_view_variant(constArr); - - using VarType = std::variant, ArrayView>; - - using ConstVarType = - std::variant, ArrayView>; + // SECTION("default variants") { + // auto var = make_view_variant(arr); + // auto constVar = make_view_variant(constArr); - EXPECT((std::is_same_v)); - EXPECT((std::is_same_v)); - } + // using VarType = std::variant, ArrayView, + // ArrayView, ArrayView, + // ArrayView, ArrayView>; - SECTION("mismatched array and variant") { - // Array is of value-type double. - using ValueList = Values; - using RankList = Ranks<1>; + // using ConstVarType = + // std::variant, ArrayView, + // ArrayView, ArrayView, + // ArrayView, ArrayView>; - EXPECT_THROWS((make_view_variant(arr))); - EXPECT_THROWS((make_view_variant(constArr))); - } + // EXPECT((std::is_same_v)); + // EXPECT((std::is_same_v)); + // } } } // namespace test From c3638d38d03069f16d573f5fef1fd611d78ffdfc Mon Sep 17 00:00:00 2001 From: odlomax Date: Thu, 12 Sep 2024 13:15:29 +0100 Subject: [PATCH 04/19] More refactoring. --- src/atlas/array/ArrayViewVariant.cc | 37 ++++++++++++---------------- src/atlas/array/ArrayViewVariant.h | 38 +++++++++++++---------------- 2 files changed, 32 insertions(+), 43 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.cc b/src/atlas/array/ArrayViewVariant.cc index af59fd3d2..fb0db8fa5 100644 --- a/src/atlas/array/ArrayViewVariant.cc +++ b/src/atlas/array/ArrayViewVariant.cc @@ -20,39 +20,32 @@ namespace { // Match array.rank() and array.datatype() to variant types. Return result of // makeView on a successfull pattern match. -template -struct MakeViewHelper { - template - static ArrayViewVariant apply(ArrayType& array, const MakeView& makeView) { - using View = std::variant_alternative_t; - using Value = typename View::value_type; - constexpr auto Rank = View::rank(); - - if (array.datatype() == DataType::kind() && array.rank() == Rank) { - return makeView(array, Value{}, std::integral_constant{}); - } - return MakeViewHelper::apply(array, makeView); +template +ArrayViewVariant executeMakeView(ArrayType& array, const MakeView& makeView) { + using View = std::variant_alternative_t; + using Value = typename View::value_type; + constexpr auto Rank = View::rank(); + + if (array.datatype() == DataType::kind() && array.rank() == Rank) { + return makeView(array, Value{}, std::integral_constant{}); } -}; -// End recursion. -template <> -struct MakeViewHelper> { - template - static ArrayViewVariant apply(ArrayType& array, const MakeView& makeView) { + if constexpr (TypeIndex < std::variant_size_v - 1) { + return executeMakeView(array, makeView); + } else { throw_Exception("ArrayView<" + array.datatype().str() + ", " + std::to_string(array.rank()) + "> is not an alternative in ArrayViewVariant.", Here()); } -}; +} template ArrayViewVariant makeViewVariantImpl(ArrayType& array) { const auto makeView = [](auto& array, auto value, auto rank) { return make_view(array); }; - return MakeViewHelper<0>::apply(array, makeView); + return executeMakeView<>(array, makeView); } template @@ -60,7 +53,7 @@ ArrayViewVariant makeHostViewVariantImpl(ArrayType& array) { const auto makeView = [](auto& array, auto value, auto rank) { return make_host_view(array); }; - return MakeViewHelper<0>::apply(array, makeView); + return executeMakeView<>(array, makeView); } template @@ -68,7 +61,7 @@ ArrayViewVariant makeDeviceViewVariantImpl(ArrayType& array) { const auto makeView = [](auto& array, auto value, auto rank) { return make_device_view(array); }; - return MakeViewHelper<0>::apply(array, makeView); + return executeMakeView<>(array, makeView); } } // namespace diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index 5fc2f9270..f369636fa 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -20,18 +20,18 @@ namespace detail { using namespace array; // Container struct for a list of types. -template -struct TypeList {}; +template +struct Types {}; // Container struct for a list of integers. -template -struct IntList {}; +template +struct Ints {}; // Supported ArrayView value types. -constexpr auto ValueList = TypeList{}; +constexpr auto Values = Types{}; // Supported ArrayView ranks. -constexpr auto RankList = IntList<1, 2, 3, 4, 5, 6, 7, 8, 9>{}; +constexpr auto Ranks = Ints<1, 2, 3, 4, 5, 6, 7, 8, 9>{}; // Helper struct to build an ArrayView variant from a list of value types and // and a list of ranks. @@ -40,27 +40,23 @@ struct VariantBuilder { using type = std::variant; // Make a VariantBuilder struct with a fully populated Views... argument. - template - static constexpr auto make(TypeList, IntList) { - return VariantBuilder< - Views..., ArrayView..., - ArrayView...>::make(TypeList{}, - IntList{}); - } - - // End recursion. - template - static constexpr VariantBuilder make(TypeList<>, - IntList) { - return VariantBuilder{}; + template + static constexpr auto make(Types, Ints) { + using NewBuilder = VariantBuilder..., + ArrayView...>; + if constexpr (sizeof...(Ts) > 0) { + return NewBuilder::make(Types{}, Ints{}); + } else { + return NewBuilder{}; + } } }; +constexpr auto variantHelper = VariantBuilder<>::make(Values, Ranks); } // namespace detail /// @brief Variant containing all supported ArrayView alternatives. -using ArrayViewVariant = decltype(detail::VariantBuilder<>::make( - detail::ValueList, detail::RankList))::type; +using ArrayViewVariant = decltype(detail::variantHelper)::type; /// @brief Use overloaded pattern as visitor. using eckit::Overloaded; From cbf7818df1da51fead09bbbadef0620de28ee55d Mon Sep 17 00:00:00 2001 From: odlomax Date: Fri, 13 Sep 2024 16:41:27 +0100 Subject: [PATCH 05/19] Updated test. --- src/atlas/array/ArrayViewVariant.cc | 15 +- src/atlas/array/ArrayViewVariant.h | 68 ++++---- src/tests/array/CMakeLists.txt | 4 +- src/tests/array/test_array_view_variant.cc | 172 +++++++++++++++++++++ src/tests/array/test_arrayviewvariant.cc | 110 ------------- 5 files changed, 215 insertions(+), 154 deletions(-) create mode 100644 src/tests/array/test_array_view_variant.cc delete mode 100644 src/tests/array/test_arrayviewvariant.cc diff --git a/src/atlas/array/ArrayViewVariant.cc b/src/atlas/array/ArrayViewVariant.cc index fb0db8fa5..e131214ae 100644 --- a/src/atlas/array/ArrayViewVariant.cc +++ b/src/atlas/array/ArrayViewVariant.cc @@ -23,11 +23,14 @@ namespace { template ArrayViewVariant executeMakeView(ArrayType& array, const MakeView& makeView) { using View = std::variant_alternative_t; - using Value = typename View::value_type; - constexpr auto Rank = View::rank(); - - if (array.datatype() == DataType::kind() && array.rank() == Rank) { - return makeView(array, Value{}, std::integral_constant{}); + constexpr auto Const = std::is_const_v; + + if constexpr (std::is_const_v == Const) { + using Value = typename View::non_const_value_type; + constexpr auto Rank = View::rank(); + if (array.datatype() == DataType::kind() && array.rank() == Rank) { + return makeView(array, Value{}, std::integral_constant{}); + } } if constexpr (TypeIndex < std::variant_size_v - 1) { @@ -82,7 +85,7 @@ ArrayViewVariant make_host_view_variant(const Array& array) { return makeHostViewVariantImpl(array); } -ArrayViewVariant make_devive_view_variant(Array& array) { +ArrayViewVariant make_device_view_variant(Array& array) { return makeDeviceViewVariantImpl(array); } diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index f369636fa..f2d6d94f3 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -10,7 +10,6 @@ #include #include "atlas/array.h" -#include "eckit/utils/Overloaded.h" namespace atlas { namespace array { @@ -24,60 +23,57 @@ template struct Types {}; // Container struct for a list of integers. -template +template struct Ints {}; -// Supported ArrayView value types. -constexpr auto Values = Types{}; - -// Supported ArrayView ranks. -constexpr auto Ranks = Ints<1, 2, 3, 4, 5, 6, 7, 8, 9>{}; - -// Helper struct to build an ArrayView variant from a list of value types and -// and a list of ranks. -template -struct VariantBuilder { - using type = std::variant; - - // Make a VariantBuilder struct with a fully populated Views... argument. - template - static constexpr auto make(Types, Ints) { - using NewBuilder = VariantBuilder..., - ArrayView...>; - if constexpr (sizeof...(Ts) > 0) { - return NewBuilder::make(Types{}, Ints{}); - } else { - return NewBuilder{}; - } - } +template +struct VariantHelper; + +// Recursively construct ArrayView std::variant from types Ts and Ranks Is. +template +struct VariantHelper, Types, Ints> { + using type = typename VariantHelper< + Types..., ArrayView...>, + Types, Ints>::type; +}; + +// End recursion. +template +struct VariantHelper, Types<>, Ints> { + using type = std::variant; }; -constexpr auto variantHelper = VariantBuilder<>::make(Values, Ranks); + +template +using Variant = typename VariantHelper, Values, Ranks>::type; } // namespace detail -/// @brief Variant containing all supported ArrayView alternatives. -using ArrayViewVariant = decltype(detail::variantHelper)::type; +/// @brief Supported ArrayView value types. +using Values = detail::Types; + +/// @brief Supported ArrayView ranks. +using Ranks = detail::Ints<1, 2, 3, 4, 5, 6, 7, 8, 9>; -/// @brief Use overloaded pattern as visitor. -using eckit::Overloaded; +/// @brief Variant containing all supported ArrayView alternatives. +using ArrayViewVariant = detail::Variant; /// @brief Create an ArrayView and assign to an ArrayViewVariant. -ArrayViewVariant make_view_variant(array::Array& array); +ArrayViewVariant make_view_variant(Array& array); /// @brief Create a const ArrayView and assign to an ArrayViewVariant. -ArrayViewVariant make_view_variant(const array::Array& array); +ArrayViewVariant make_view_variant(const Array& array); /// @brief Create a host ArrayView and assign to an ArrayViewVariant. -ArrayViewVariant make_host_view_variant(array::Array& array); +ArrayViewVariant make_host_view_variant(Array& array); /// @brief Create a host const ArrayView and assign to an ArrayViewVariant. -ArrayViewVariant make_host_view_variant(const array::Array& array); +ArrayViewVariant make_host_view_variant(const Array& array); /// @brief Create a device ArrayView and assign to an ArrayViewVariant. -ArrayViewVariant make_devive_view_variant(array::Array& array); +ArrayViewVariant make_device_view_variant(Array& array); /// @brief Create a const device ArrayView and assign to an ArrayViewVariant. -ArrayViewVariant make_device_view_variant(const array::Array& array); +ArrayViewVariant make_device_view_variant(const Array& array); } // namespace array } // namespace atlas diff --git a/src/tests/array/CMakeLists.txt b/src/tests/array/CMakeLists.txt index eda04a6d1..880e46ba6 100644 --- a/src/tests/array/CMakeLists.txt +++ b/src/tests/array/CMakeLists.txt @@ -81,8 +81,8 @@ atlas_add_hic_test( ENVIRONMENT ${ATLAS_TEST_ENVIRONMENT} ) -ecbuild_add_test( TARGET atlas_test_arrayviewviewvariant - SOURCES test_arrayviewvariant.cc +ecbuild_add_test( TARGET atlas_test_array_view_variant + SOURCES test_array_view_variant.cc LIBS atlas ENVIRONMENT ${ATLAS_TEST_ENVIRONMENT} ) diff --git a/src/tests/array/test_array_view_variant.cc b/src/tests/array/test_array_view_variant.cc new file mode 100644 index 000000000..6d171a0d8 --- /dev/null +++ b/src/tests/array/test_array_view_variant.cc @@ -0,0 +1,172 @@ +/* + * (C) Crown Copyright 2024 Met Office + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + */ + +#include +#include + +#include "atlas/array.h" +#include "atlas/array/ArrayViewVariant.h" +#include "eckit/utils/Overloaded.h" +#include "tests/AtlasTestEnvironment.h" + +namespace atlas { +namespace test { + +using namespace array; + +CASE("test variant assignment") { + auto array1 = array::ArrayT(2); + auto array2 = array::ArrayT(2, 3); + auto array3 = array::ArrayT(2, 3, 4); + const auto& arrayRef = array1; + + array1.allocateDevice(); + array2.allocateDevice(); + array3.allocateDevice(); + + auto view1 = make_view_variant(array1); + auto view2 = make_view_variant(array2); + auto view3 = make_view_variant(array3); + auto view4 = make_view_variant(arrayRef); + + auto hostView1 = make_host_view_variant(array1); + auto hostView2 = make_host_view_variant(array2); + auto hostView3 = make_host_view_variant(array3); + auto hostView4 = make_host_view_variant(arrayRef); + + auto deviceView1 = make_device_view_variant(array1); + auto deviceView2 = make_device_view_variant(array2); + auto deviceView3 = make_device_view_variant(array3); + auto deviceView4 = make_device_view_variant(arrayRef); + + const auto visitVariants = [](auto& var1, auto& var2, auto var3, auto var4) { + std::visit( + [](auto&& view) { + using View = std::remove_reference_t; + EXPECT_EQ(View::rank(), 1); + EXPECT((std::is_same_v)); + }, + var1); + + std::visit( + [](auto&& view) { + using View = std::remove_reference_t; + EXPECT_EQ(View::rank(), 2); + EXPECT((std::is_same_v)); + }, + var2); + + std::visit( + [](auto&& view) { + using View = std::remove_reference_t; + EXPECT_EQ(View::rank(), 3); + EXPECT((std::is_same_v)); + }, + var3); + + std::visit( + [](auto&& view) { + using View = std::remove_reference_t; + EXPECT_EQ(View::rank(), 1); + EXPECT((std::is_same_v)); + }, + var4); + }; + + visitVariants(view1, view2, view3, view4); + visitVariants(hostView1, hostView2, hostView3, hostView4); + visitVariants(deviceView1, deviceView2, deviceView3, deviceView4); +} + +template +constexpr auto Rank = std::decay_t::rank(); + +CASE("test std::visit") { + auto array1 = ArrayT(10); + make_view(array1).assign({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); + + auto array2 = ArrayT(5, 2); + make_view(array2).assign({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); + + const auto testRank1 = [](auto&& view) { + using View = std::decay_t; + EXPECT_EQ(View::rank(), 1); + using Value = typename View::value_type; + EXPECT((std::is_same_v)); + + for (auto i = size_t{0}; i < view.size(); ++i) { + EXPECT_EQ(view(i), static_cast(i)); + } + }; + + const auto testRank2 = [](auto&& view) { + using View = std::decay_t; + EXPECT_EQ(View::rank(), 2); + using Value = typename View::value_type; + EXPECT((std::is_same_v)); + + auto testValue = int{0}; + for (auto i = idx_t{0}; i < view.shape(0); ++i) { + for (auto j = idx_t{0}; j < view.shape(1); ++j) { + EXPECT_EQ(view(i, j), static_cast(testValue++)); + } + } + }; + + const auto var1 = make_view_variant(array1); + const auto var2 = make_view_variant(array2); + + SECTION("demonstrate 'if constexpr' pattern") { + auto rank1Tested = false; + auto rank2Tested = false; + + const auto visitor = [&](auto&& view) { + if constexpr (Rank == 1) { + testRank1(view); + rank1Tested = true; + } + if constexpr (Rank == 2) { + testRank2(view); + rank2Tested = true; + } + }; + + std::visit(visitor, var1); + EXPECT(rank1Tested); + std::visit(visitor, var2); + EXPECT(rank2Tested); + } + + SECTION("demonstrate 'overloaded' pattern") { + // Note, SFINAE can be eliminated using concepts and explicit lambda + // templates in C++20. + auto rank1Tested = false; + auto rank2Tested = false; + const auto visitor = eckit::Overloaded{ + [&](auto&& view) -> std::enable_if_t == 1> { + testRank1(view); + rank1Tested = true; + }, + [&](auto&& view) -> std::enable_if_t == 2> { + testRank2(view); + rank2Tested = true; + }, + [](auto&& view) -> std::enable_if_t >= 3> { + // do nothing. + }}; + + std::visit(visitor, var1); + EXPECT(rank1Tested); + std::visit(visitor, var2); + EXPECT(rank2Tested); + } +} + +} // namespace test +} // namespace atlas + +int main(int argc, char** argv) { return atlas::test::run(argc, argv); } diff --git a/src/tests/array/test_arrayviewvariant.cc b/src/tests/array/test_arrayviewvariant.cc deleted file mode 100644 index 2ab24bd2c..000000000 --- a/src/tests/array/test_arrayviewvariant.cc +++ /dev/null @@ -1,110 +0,0 @@ -/* - * (C) Crown Copyright 2024 Met Office - * - * This software is licensed under the terms of the Apache Licence Version 2.0 - * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. - */ - -#include -#include -#include - -#include "atlas/array.h" -#include "atlas/array/ArrayViewVariant.h" -#include "tests/AtlasTestEnvironment.h" - -namespace atlas { -namespace test { - -using namespace array; - -CASE("test visit") { - auto arr1 = array::ArrayT(2); - auto arr2 = array::ArrayT(2, 3); - auto arr3 = array::ArrayT(2, 3, 4); - - const auto var1 = make_view_variant(arr1); - const auto var2 = make_view_variant(arr2); - const auto var3 = make_view_variant(arr3); - - std::visit( - [](auto&& view) { - using View = std::remove_reference_t; - EXPECT_EQ(View::rank(), 1); - EXPECT((std::is_same_v)); - }, - var1); - - std::visit( - [](auto&& view) { - using View = std::remove_reference_t; - EXPECT_EQ(View::rank(), 2); - EXPECT((std::is_same_v)); - }, - var2); - - std::visit( - [](auto&& view) { - using View = std::remove_reference_t; - EXPECT_EQ(View::rank(), 3); - EXPECT((std::is_same_v)); - }, - var3); -} - -template -constexpr auto Rank = std::decay_t::rank(); - -CASE("test array view data") { - auto arr = ArrayT(10); - make_view(arr).assign({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); - - const auto& arrRef = arr; - const auto var = make_view_variant(arrRef); - - const auto visitor = Overloaded{ - [](auto&& view) -> std::enable_if_t == 1> { - using View = std::decay_t; - EXPECT_EQ(View::rank(), 1); - using Value = typename View::value_type; - EXPECT((std::is_same_v)); - - for (auto i = size_t{0}; i < view.size(); ++i) { - EXPECT_EQ(view(i), static_cast(i)); - } - }, - [](auto&& view) -> std::enable_if_t != 1> { - // do nothing. - }}; - - std::visit(visitor, var); -} - -CASE("test instantiation") { - auto arr = array::ArrayT(1); - const auto constArr = array::ArrayT(1); - - // SECTION("default variants") { - // auto var = make_view_variant(arr); - // auto constVar = make_view_variant(constArr); - - // using VarType = std::variant, ArrayView, - // ArrayView, ArrayView, - // ArrayView, ArrayView>; - - // using ConstVarType = - // std::variant, ArrayView, - // ArrayView, ArrayView, - // ArrayView, ArrayView>; - - // EXPECT((std::is_same_v)); - // EXPECT((std::is_same_v)); - // } -} - -} // namespace test -} // namespace atlas - -int main(int argc, char** argv) { return atlas::test::run(argc, argv); } From b6ff4af76b000bf400c79b3989a8d4c6ad0a6b77 Mon Sep 17 00:00:00 2001 From: odlomax Date: Fri, 13 Sep 2024 20:08:56 +0100 Subject: [PATCH 06/19] Attempting to address gnu 7.3 compiler errors. --- src/atlas/array/ArrayViewVariant.h | 2 +- src/tests/array/test_array_view_variant.cc | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index f2d6d94f3..c8e6c922c 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -29,7 +29,7 @@ struct Ints {}; template struct VariantHelper; -// Recursively construct ArrayView std::variant from types Ts and Ranks Is. +// Recursively construct ArrayView std::variant from types Ts and ranks Is. template struct VariantHelper, Types, Ints> { using type = typename VariantHelper< diff --git a/src/tests/array/test_array_view_variant.cc b/src/tests/array/test_array_view_variant.cc index 6d171a0d8..f9c848ee3 100644 --- a/src/tests/array/test_array_view_variant.cc +++ b/src/tests/array/test_array_view_variant.cc @@ -83,7 +83,9 @@ CASE("test variant assignment") { } template -constexpr auto Rank = std::decay_t::rank(); +constexpr auto Rank() { + return std::remove_reference_t::rank(); +} CASE("test std::visit") { auto array1 = ArrayT(10); @@ -93,7 +95,7 @@ CASE("test std::visit") { make_view(array2).assign({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); const auto testRank1 = [](auto&& view) { - using View = std::decay_t; + using View = std::remove_reference_t; EXPECT_EQ(View::rank(), 1); using Value = typename View::value_type; EXPECT((std::is_same_v)); @@ -104,7 +106,7 @@ CASE("test std::visit") { }; const auto testRank2 = [](auto&& view) { - using View = std::decay_t; + using View = std::remove_reference_t; EXPECT_EQ(View::rank(), 2); using Value = typename View::value_type; EXPECT((std::is_same_v)); @@ -125,11 +127,11 @@ CASE("test std::visit") { auto rank2Tested = false; const auto visitor = [&](auto&& view) { - if constexpr (Rank == 1) { + if constexpr (Rank() == 1) { testRank1(view); rank1Tested = true; } - if constexpr (Rank == 2) { + if constexpr (Rank() == 2) { testRank2(view); rank2Tested = true; } @@ -147,15 +149,15 @@ CASE("test std::visit") { auto rank1Tested = false; auto rank2Tested = false; const auto visitor = eckit::Overloaded{ - [&](auto&& view) -> std::enable_if_t == 1> { + [&](auto&& view) -> std::enable_if_t() == 1> { testRank1(view); rank1Tested = true; }, - [&](auto&& view) -> std::enable_if_t == 2> { + [&](auto&& view) -> std::enable_if_t() == 2> { testRank2(view); rank2Tested = true; }, - [](auto&& view) -> std::enable_if_t >= 3> { + [](auto&& view) -> std::enable_if_t<(Rank() > 2)> { // do nothing. }}; From 6db4f8382380f94c24d23d989ecfde1cbdfc0d1f Mon Sep 17 00:00:00 2001 From: odlomax Date: Fri, 13 Sep 2024 20:25:18 +0100 Subject: [PATCH 07/19] Typos in comments. --- src/atlas/array/ArrayViewVariant.cc | 2 +- src/atlas/array/ArrayViewVariant.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.cc b/src/atlas/array/ArrayViewVariant.cc index e131214ae..547893e06 100644 --- a/src/atlas/array/ArrayViewVariant.cc +++ b/src/atlas/array/ArrayViewVariant.cc @@ -19,7 +19,7 @@ using namespace detail; namespace { // Match array.rank() and array.datatype() to variant types. Return result of -// makeView on a successfull pattern match. +// makeView on a successful pattern match. template ArrayViewVariant executeMakeView(ArrayType& array, const MakeView& makeView) { using View = std::variant_alternative_t; diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index c8e6c922c..101157703 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -66,7 +66,7 @@ ArrayViewVariant make_view_variant(const Array& array); /// @brief Create a host ArrayView and assign to an ArrayViewVariant. ArrayViewVariant make_host_view_variant(Array& array); -/// @brief Create a host const ArrayView and assign to an ArrayViewVariant. +/// @brief Create a const host ArrayView and assign to an ArrayViewVariant. ArrayViewVariant make_host_view_variant(const Array& array); /// @brief Create a device ArrayView and assign to an ArrayViewVariant. From f4d369736c78033fec5bc530f1d8355b452fe61e Mon Sep 17 00:00:00 2001 From: odlomax Date: Mon, 16 Sep 2024 12:11:20 +0100 Subject: [PATCH 08/19] Added missing EXPECTs in test. --- src/atlas/array/ArrayViewVariant.cc | 1 + src/tests/array/test_array_view_variant.cc | 9 ++++++--- src/tests/util/CMakeLists.txt | 1 - 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.cc b/src/atlas/array/ArrayViewVariant.cc index 547893e06..28d532117 100644 --- a/src/atlas/array/ArrayViewVariant.cc +++ b/src/atlas/array/ArrayViewVariant.cc @@ -8,6 +8,7 @@ #include "atlas/array/ArrayViewVariant.h" #include +#include #include "atlas/runtime/Exception.h" diff --git a/src/tests/array/test_array_view_variant.cc b/src/tests/array/test_array_view_variant.cc index f9c848ee3..9f60a10be 100644 --- a/src/tests/array/test_array_view_variant.cc +++ b/src/tests/array/test_array_view_variant.cc @@ -128,13 +128,15 @@ CASE("test std::visit") { const auto visitor = [&](auto&& view) { if constexpr (Rank() == 1) { - testRank1(view); rank1Tested = true; + return testRank1(view); } if constexpr (Rank() == 2) { - testRank2(view); rank2Tested = true; + return testRank2(view); } + // Test should not reach here. + EXPECT(false); }; std::visit(visitor, var1); @@ -158,7 +160,8 @@ CASE("test std::visit") { rank2Tested = true; }, [](auto&& view) -> std::enable_if_t<(Rank() > 2)> { - // do nothing. + // Test should not reach here. + EXPECT(false); }}; std::visit(visitor, var1); diff --git a/src/tests/util/CMakeLists.txt b/src/tests/util/CMakeLists.txt index 703d310fd..47d8b6def 100644 --- a/src/tests/util/CMakeLists.txt +++ b/src/tests/util/CMakeLists.txt @@ -94,4 +94,3 @@ ecbuild_add_test( TARGET atlas_test_unitsphere ENVIRONMENT ${ATLAS_TEST_ENVIRONMENT} ) - From ed15c8421d1f7ac6df437a515d037752cd342661 Mon Sep 17 00:00:00 2001 From: odlomax Date: Mon, 16 Sep 2024 15:21:01 +0100 Subject: [PATCH 09/19] Refactored detial::VariantHelper template. --- src/atlas/array/ArrayViewVariant.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index 101157703..d5dd196f8 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -26,25 +26,25 @@ struct Types {}; template struct Ints {}; -template +template struct VariantHelper; // Recursively construct ArrayView std::variant from types Ts and ranks Is. -template -struct VariantHelper, Types, Ints> { - using type = typename VariantHelper< - Types..., ArrayView...>, - Types, Ints>::type; +template +struct VariantHelper, Ints, ArrayViews...> { + using type = typename VariantHelper, Ints, ArrayViews..., + ArrayView..., + ArrayView...>::type; }; // End recursion. -template -struct VariantHelper, Types<>, Ints> { +template +struct VariantHelper, Ints, ArrayViews...> { using type = std::variant; }; template -using Variant = typename VariantHelper, Values, Ranks>::type; +using Variant = typename VariantHelper::type; } // namespace detail From aeb352f1106bcd9fec00d1162ec1000ae827f9e7 Mon Sep 17 00:00:00 2001 From: odlomax Date: Wed, 25 Sep 2024 16:11:00 +0100 Subject: [PATCH 10/19] Merged in ArrayViewVariant refactor. --- src/atlas/array.h | 1 + src/atlas/array/ArrayViewVariant.cc | 46 ++++++++++++++-------- src/atlas/array/ArrayViewVariant.h | 29 +++++++++++--- src/tests/array/test_array_view_variant.cc | 15 +++---- 4 files changed, 58 insertions(+), 33 deletions(-) diff --git a/src/atlas/array.h b/src/atlas/array.h index c2cf7f720..a7ac48d07 100644 --- a/src/atlas/array.h +++ b/src/atlas/array.h @@ -23,6 +23,7 @@ #include "atlas/array/ArraySpec.h" #include "atlas/array/ArrayStrides.h" #include "atlas/array/ArrayView.h" +#include "atlas/array/ArrayViewVariant.h" #include "atlas/array/DataType.h" #include "atlas/array/LocalView.h" #include "atlas/array/MakeView.h" diff --git a/src/atlas/array/ArrayViewVariant.cc b/src/atlas/array/ArrayViewVariant.cc index 28d532117..f62efd2d8 100644 --- a/src/atlas/array/ArrayViewVariant.cc +++ b/src/atlas/array/ArrayViewVariant.cc @@ -19,22 +19,34 @@ using namespace detail; namespace { +template +struct VariantTypeHelper { + using type = ArrayViewVariant; +}; + +template <> +struct VariantTypeHelper { + using type = ConstArrayViewVariant; +}; + +template +using VariantType = + typename VariantTypeHelper>::type; + // Match array.rank() and array.datatype() to variant types. Return result of // makeView on a successful pattern match. template -ArrayViewVariant executeMakeView(ArrayType& array, const MakeView& makeView) { - using View = std::variant_alternative_t; - constexpr auto Const = std::is_const_v; - - if constexpr (std::is_const_v == Const) { - using Value = typename View::non_const_value_type; - constexpr auto Rank = View::rank(); - if (array.datatype() == DataType::kind() && array.rank() == Rank) { - return makeView(array, Value{}, std::integral_constant{}); - } +VariantType executeMakeView(ArrayType& array, + const MakeView& makeView) { + using View = std::variant_alternative_t>; + using Value = typename View::non_const_value_type; + constexpr auto Rank = View::rank(); + + if (array.datatype() == DataType::kind() && array.rank() == Rank) { + return makeView(array, Value{}, std::integral_constant{}); } - if constexpr (TypeIndex < std::variant_size_v - 1) { + if constexpr (TypeIndex < std::variant_size_v> - 1) { return executeMakeView(array, makeView); } else { throw_Exception("ArrayView<" + array.datatype().str() + ", " + @@ -45,7 +57,7 @@ ArrayViewVariant executeMakeView(ArrayType& array, const MakeView& makeView) { } template -ArrayViewVariant makeViewVariantImpl(ArrayType& array) { +VariantType makeViewVariantImpl(ArrayType& array) { const auto makeView = [](auto& array, auto value, auto rank) { return make_view(array); }; @@ -53,7 +65,7 @@ ArrayViewVariant makeViewVariantImpl(ArrayType& array) { } template -ArrayViewVariant makeHostViewVariantImpl(ArrayType& array) { +VariantType makeHostViewVariantImpl(ArrayType& array) { const auto makeView = [](auto& array, auto value, auto rank) { return make_host_view(array); }; @@ -61,7 +73,7 @@ ArrayViewVariant makeHostViewVariantImpl(ArrayType& array) { } template -ArrayViewVariant makeDeviceViewVariantImpl(ArrayType& array) { +VariantType makeDeviceViewVariantImpl(ArrayType& array) { const auto makeView = [](auto& array, auto value, auto rank) { return make_device_view(array); }; @@ -74,7 +86,7 @@ ArrayViewVariant make_view_variant(Array& array) { return makeViewVariantImpl(array); } -ArrayViewVariant make_view_variant(const Array& array) { +ConstArrayViewVariant make_view_variant(const Array& array) { return makeViewVariantImpl(array); } @@ -82,7 +94,7 @@ ArrayViewVariant make_host_view_variant(Array& array) { return makeHostViewVariantImpl(array); } -ArrayViewVariant make_host_view_variant(const Array& array) { +ConstArrayViewVariant make_host_view_variant(const Array& array) { return makeHostViewVariantImpl(array); } @@ -90,7 +102,7 @@ ArrayViewVariant make_device_view_variant(Array& array) { return makeDeviceViewVariantImpl(array); } -ArrayViewVariant make_device_view_variant(const Array& array) { +ConstArrayViewVariant make_device_view_variant(const Array& array) { return makeDeviceViewVariantImpl(array); } diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index d5dd196f8..0ea3f87c6 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -20,7 +20,9 @@ using namespace array; // Container struct for a list of types. template -struct Types {}; +struct Types { + using add_const = Types...>; +}; // Container struct for a list of integers. template @@ -33,7 +35,6 @@ struct VariantHelper; template struct VariantHelper, Ints, ArrayViews...> { using type = typename VariantHelper, Ints, ArrayViews..., - ArrayView..., ArrayView...>::type; }; @@ -54,26 +55,42 @@ using Values = detail::Types; /// @brief Supported ArrayView ranks. using Ranks = detail::Ints<1, 2, 3, 4, 5, 6, 7, 8, 9>; -/// @brief Variant containing all supported ArrayView alternatives. +/// @brief Variant containing all supported non-const ArrayView alternatives. using ArrayViewVariant = detail::Variant; +/// @brief Variant containing all supported const ArrayView alternatives. +using ConstArrayViewVariant = detail::Variant; + /// @brief Create an ArrayView and assign to an ArrayViewVariant. ArrayViewVariant make_view_variant(Array& array); /// @brief Create a const ArrayView and assign to an ArrayViewVariant. -ArrayViewVariant make_view_variant(const Array& array); +ConstArrayViewVariant make_view_variant(const Array& array); /// @brief Create a host ArrayView and assign to an ArrayViewVariant. ArrayViewVariant make_host_view_variant(Array& array); /// @brief Create a const host ArrayView and assign to an ArrayViewVariant. -ArrayViewVariant make_host_view_variant(const Array& array); +ConstArrayViewVariant make_host_view_variant(const Array& array); /// @brief Create a device ArrayView and assign to an ArrayViewVariant. ArrayViewVariant make_device_view_variant(Array& array); /// @brief Create a const device ArrayView and assign to an ArrayViewVariant. -ArrayViewVariant make_device_view_variant(const Array& array); +ConstArrayViewVariant make_device_view_variant(const Array& array); + +/// @brief Return true if ArrayView::rank() is any of Ranks... +template +constexpr bool RankIs() { + return ((std::decay_t::rank() == Ranks) || ...); +} + +/// @brief Return true if View::non_const_value_type is any of Values... +template +constexpr bool ValueIs() { + using Value = typename std::decay_t::non_const_value_type; + return ((std::is_same_v) || ...); +} } // namespace array } // namespace atlas diff --git a/src/tests/array/test_array_view_variant.cc b/src/tests/array/test_array_view_variant.cc index 9f60a10be..29a3cb044 100644 --- a/src/tests/array/test_array_view_variant.cc +++ b/src/tests/array/test_array_view_variant.cc @@ -82,11 +82,6 @@ CASE("test variant assignment") { visitVariants(deviceView1, deviceView2, deviceView3, deviceView4); } -template -constexpr auto Rank() { - return std::remove_reference_t::rank(); -} - CASE("test std::visit") { auto array1 = ArrayT(10); make_view(array1).assign({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); @@ -127,11 +122,11 @@ CASE("test std::visit") { auto rank2Tested = false; const auto visitor = [&](auto&& view) { - if constexpr (Rank() == 1) { + if constexpr (RankIs()) { rank1Tested = true; return testRank1(view); } - if constexpr (Rank() == 2) { + if constexpr (RankIs()) { rank2Tested = true; return testRank2(view); } @@ -151,15 +146,15 @@ CASE("test std::visit") { auto rank1Tested = false; auto rank2Tested = false; const auto visitor = eckit::Overloaded{ - [&](auto&& view) -> std::enable_if_t() == 1> { + [&](auto&& view) -> std::enable_if_t()> { testRank1(view); rank1Tested = true; }, - [&](auto&& view) -> std::enable_if_t() == 2> { + [&](auto&& view) -> std::enable_if_t()> { testRank2(view); rank2Tested = true; }, - [](auto&& view) -> std::enable_if_t<(Rank() > 2)> { + [](auto&& view) -> std::enable_if_t()> { // Test should not reach here. EXPECT(false); }}; From d7743eed77172f4479615d2466903912e0446599 Mon Sep 17 00:00:00 2001 From: odlomax Date: Wed, 2 Oct 2024 10:24:33 +0100 Subject: [PATCH 11/19] Refactored introspection helpers. --- src/atlas/array/ArrayViewVariant.h | 27 ++++++---- src/tests/array/test_array_view_variant.cc | 57 ++++++++++++---------- 2 files changed, 49 insertions(+), 35 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index 0ea3f87c6..a32a90539 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -50,16 +50,16 @@ using Variant = typename VariantHelper::type; } // namespace detail /// @brief Supported ArrayView value types. -using Values = detail::Types; +using ValueTypes = detail::Types; /// @brief Supported ArrayView ranks. using Ranks = detail::Ints<1, 2, 3, 4, 5, 6, 7, 8, 9>; /// @brief Variant containing all supported non-const ArrayView alternatives. -using ArrayViewVariant = detail::Variant; +using ArrayViewVariant = detail::Variant; /// @brief Variant containing all supported const ArrayView alternatives. -using ConstArrayViewVariant = detail::Variant; +using ConstArrayViewVariant = detail::Variant; /// @brief Create an ArrayView and assign to an ArrayViewVariant. ArrayViewVariant make_view_variant(Array& array); @@ -79,17 +79,24 @@ ArrayViewVariant make_device_view_variant(Array& array); /// @brief Create a const device ArrayView and assign to an ArrayViewVariant. ConstArrayViewVariant make_device_view_variant(const Array& array); -/// @brief Return true if ArrayView::rank() is any of Ranks... +/// @brief Return true if View::rank() is any of Ranks... template -constexpr bool RankIs() { +constexpr bool is_rank() { return ((std::decay_t::rank() == Ranks) || ...); } -/// @brief Return true if View::non_const_value_type is any of Values... -template -constexpr bool ValueIs() { - using Value = typename std::decay_t::non_const_value_type; - return ((std::is_same_v) || ...); +/// @brief Return true if View::value_type is any of ValuesTypes... +template +constexpr bool is_value_type() { + using ValueType = typename std::decay_t::value_type; + return ((std::is_same_v) || ...); +} + +/// @brief Return true if View::non_const_value_type is any of ValuesTypes... +template +constexpr bool is_non_const_value_type() { + using ValueType = typename std::decay_t::non_const_value_type; + return ((std::is_same_v) || ...); } } // namespace array diff --git a/src/tests/array/test_array_view_variant.cc b/src/tests/array/test_array_view_variant.cc index 29a3cb044..a052d234f 100644 --- a/src/tests/array/test_array_view_variant.cc +++ b/src/tests/array/test_array_view_variant.cc @@ -33,10 +33,10 @@ CASE("test variant assignment") { auto view3 = make_view_variant(array3); auto view4 = make_view_variant(arrayRef); - auto hostView1 = make_host_view_variant(array1); - auto hostView2 = make_host_view_variant(array2); - auto hostView3 = make_host_view_variant(array3); - auto hostView4 = make_host_view_variant(arrayRef); + const auto hostView1 = make_host_view_variant(array1); + const auto hostView2 = make_host_view_variant(array2); + const auto hostView3 = make_host_view_variant(array3); + const auto hostView4 = make_host_view_variant(arrayRef); auto deviceView1 = make_device_view_variant(array1); auto deviceView2 = make_device_view_variant(array2); @@ -46,33 +46,37 @@ CASE("test variant assignment") { const auto visitVariants = [](auto& var1, auto& var2, auto var3, auto var4) { std::visit( [](auto&& view) { - using View = std::remove_reference_t; - EXPECT_EQ(View::rank(), 1); - EXPECT((std::is_same_v)); + using View = decltype(view); + EXPECT((is_rank())); + EXPECT((is_value_type())); + EXPECT((is_non_const_value_type())); }, var1); std::visit( [](auto&& view) { - using View = std::remove_reference_t; - EXPECT_EQ(View::rank(), 2); - EXPECT((std::is_same_v)); + using View = decltype(view); + EXPECT((is_rank())); + EXPECT((is_value_type())); + EXPECT((is_non_const_value_type())); }, var2); std::visit( [](auto&& view) { - using View = std::remove_reference_t; - EXPECT_EQ(View::rank(), 3); - EXPECT((std::is_same_v)); + using View = decltype(view); + EXPECT((is_rank())); + EXPECT((is_value_type())); + EXPECT((is_non_const_value_type())); }, var3); std::visit( [](auto&& view) { - using View = std::remove_reference_t; - EXPECT_EQ(View::rank(), 1); - EXPECT((std::is_same_v)); + using View = decltype(view); + EXPECT((is_rank())); + EXPECT((is_value_type())); + EXPECT((is_non_const_value_type())); }, var4); }; @@ -90,11 +94,13 @@ CASE("test std::visit") { make_view(array2).assign({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); const auto testRank1 = [](auto&& view) { + + using View = std::remove_reference_t; - EXPECT_EQ(View::rank(), 1); using Value = typename View::value_type; - EXPECT((std::is_same_v)); + EXPECT((is_rank())); + EXPECT((is_value_type())); for (auto i = size_t{0}; i < view.size(); ++i) { EXPECT_EQ(view(i), static_cast(i)); } @@ -102,9 +108,10 @@ CASE("test std::visit") { const auto testRank2 = [](auto&& view) { using View = std::remove_reference_t; - EXPECT_EQ(View::rank(), 2); using Value = typename View::value_type; - EXPECT((std::is_same_v)); + + EXPECT((is_rank())); + EXPECT((is_value_type())); auto testValue = int{0}; for (auto i = idx_t{0}; i < view.shape(0); ++i) { @@ -122,11 +129,11 @@ CASE("test std::visit") { auto rank2Tested = false; const auto visitor = [&](auto&& view) { - if constexpr (RankIs()) { + if constexpr (is_rank()) { rank1Tested = true; return testRank1(view); } - if constexpr (RankIs()) { + if constexpr (is_rank()) { rank2Tested = true; return testRank2(view); } @@ -146,15 +153,15 @@ CASE("test std::visit") { auto rank1Tested = false; auto rank2Tested = false; const auto visitor = eckit::Overloaded{ - [&](auto&& view) -> std::enable_if_t()> { + [&](auto&& view) -> std::enable_if_t()> { testRank1(view); rank1Tested = true; }, - [&](auto&& view) -> std::enable_if_t()> { + [&](auto&& view) -> std::enable_if_t()> { testRank2(view); rank2Tested = true; }, - [](auto&& view) -> std::enable_if_t()> { + [](auto&& view) -> std::enable_if_t()> { // Test should not reach here. EXPECT(false); }}; From 61db7f899fff3369eddf93dfb52882786822390e Mon Sep 17 00:00:00 2001 From: odlomax Date: Wed, 2 Oct 2024 14:27:08 +0100 Subject: [PATCH 12/19] Refactor helper function signatures. Removed SFINAE test. --- src/atlas/array/ArrayViewVariant.h | 13 +-- src/tests/array/test_array_view_variant.cc | 130 +++++++-------------- 2 files changed, 48 insertions(+), 95 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index a32a90539..7bd33b15e 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -80,21 +80,20 @@ ArrayViewVariant make_device_view_variant(Array& array); ConstArrayViewVariant make_device_view_variant(const Array& array); /// @brief Return true if View::rank() is any of Ranks... -template -constexpr bool is_rank() { +template +constexpr bool is_rank(const View&) { return ((std::decay_t::rank() == Ranks) || ...); } - /// @brief Return true if View::value_type is any of ValuesTypes... -template -constexpr bool is_value_type() { +template +constexpr bool is_value_type(const View&) { using ValueType = typename std::decay_t::value_type; return ((std::is_same_v) || ...); } /// @brief Return true if View::non_const_value_type is any of ValuesTypes... -template -constexpr bool is_non_const_value_type() { +template +constexpr bool is_non_const_value_type(const View&) { using ValueType = typename std::decay_t::non_const_value_type; return ((std::is_same_v) || ...); } diff --git a/src/tests/array/test_array_view_variant.cc b/src/tests/array/test_array_view_variant.cc index a052d234f..d870ed092 100644 --- a/src/tests/array/test_array_view_variant.cc +++ b/src/tests/array/test_array_view_variant.cc @@ -43,40 +43,38 @@ CASE("test variant assignment") { auto deviceView3 = make_device_view_variant(array3); auto deviceView4 = make_device_view_variant(arrayRef); + auto view = make_view(array1); + const auto visitVariants = [](auto& var1, auto& var2, auto var3, auto var4) { std::visit( [](auto&& view) { - using View = decltype(view); - EXPECT((is_rank())); - EXPECT((is_value_type())); - EXPECT((is_non_const_value_type())); + EXPECT((is_rank<1>(view))); + EXPECT((is_value_type(view))); + EXPECT((is_non_const_value_type(view))); }, var1); std::visit( [](auto&& view) { - using View = decltype(view); - EXPECT((is_rank())); - EXPECT((is_value_type())); - EXPECT((is_non_const_value_type())); + EXPECT((is_rank<2>(view))); + EXPECT((is_value_type(view))); + EXPECT((is_non_const_value_type(view))); }, var2); std::visit( [](auto&& view) { - using View = decltype(view); - EXPECT((is_rank())); - EXPECT((is_value_type())); - EXPECT((is_non_const_value_type())); + EXPECT((is_rank<3>(view))); + EXPECT((is_value_type(view))); + EXPECT((is_non_const_value_type(view))); }, var3); std::visit( [](auto&& view) { - using View = decltype(view); - EXPECT((is_rank())); - EXPECT((is_value_type())); - EXPECT((is_non_const_value_type())); + EXPECT((is_rank<1>(view))); + EXPECT((is_value_type(view))); + EXPECT((is_non_const_value_type(view))); }, var4); }; @@ -93,84 +91,40 @@ CASE("test std::visit") { auto array2 = ArrayT(5, 2); make_view(array2).assign({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}); - const auto testRank1 = [](auto&& view) { - - - using View = std::remove_reference_t; - using Value = typename View::value_type; - - EXPECT((is_rank())); - EXPECT((is_value_type())); - for (auto i = size_t{0}; i < view.size(); ++i) { - EXPECT_EQ(view(i), static_cast(i)); - } - }; - - const auto testRank2 = [](auto&& view) { - using View = std::remove_reference_t; - using Value = typename View::value_type; - - EXPECT((is_rank())); - EXPECT((is_value_type())); - - auto testValue = int{0}; - for (auto i = idx_t{0}; i < view.shape(0); ++i) { - for (auto j = idx_t{0}; j < view.shape(1); ++j) { - EXPECT_EQ(view(i, j), static_cast(testValue++)); - } - } - }; - const auto var1 = make_view_variant(array1); const auto var2 = make_view_variant(array2); - - SECTION("demonstrate 'if constexpr' pattern") { - auto rank1Tested = false; - auto rank2Tested = false; - - const auto visitor = [&](auto&& view) { - if constexpr (is_rank()) { - rank1Tested = true; - return testRank1(view); + auto rank1Tested = false; + auto rank2Tested = false; + + const auto visitor = [&](auto&& view) { + if constexpr (is_rank<1>(view)) { + EXPECT((is_value_type(view))); + auto testValue = int{0}; + for (auto i = size_t{0}; i < view.size(); ++i) { + const auto value = view(i); + EXPECT_EQ(value, static_cast(testValue++)); } - if constexpr (is_rank()) { - rank2Tested = true; - return testRank2(view); + rank1Tested = true; + } else if constexpr (is_rank<2>(view)) { + EXPECT((is_value_type(view))); + auto testValue = int{0}; + for (auto i = idx_t{0}; i < view.shape(0); ++i) { + for (auto j = idx_t{0}; j < view.shape(1); ++j) { + const auto value = view(i, j); + EXPECT_EQ(value, static_cast(testValue++)); + } } + rank2Tested = true; + } else { // Test should not reach here. EXPECT(false); - }; - - std::visit(visitor, var1); - EXPECT(rank1Tested); - std::visit(visitor, var2); - EXPECT(rank2Tested); - } - - SECTION("demonstrate 'overloaded' pattern") { - // Note, SFINAE can be eliminated using concepts and explicit lambda - // templates in C++20. - auto rank1Tested = false; - auto rank2Tested = false; - const auto visitor = eckit::Overloaded{ - [&](auto&& view) -> std::enable_if_t()> { - testRank1(view); - rank1Tested = true; - }, - [&](auto&& view) -> std::enable_if_t()> { - testRank2(view); - rank2Tested = true; - }, - [](auto&& view) -> std::enable_if_t()> { - // Test should not reach here. - EXPECT(false); - }}; - - std::visit(visitor, var1); - EXPECT(rank1Tested); - std::visit(visitor, var2); - EXPECT(rank2Tested); - } + } + }; + + std::visit(visitor, var1); + EXPECT(rank1Tested); + std::visit(visitor, var2); + EXPECT(rank2Tested); } } // namespace test From df696e675ec183c6fa66ac6bf2ab4867e8fca3f7 Mon Sep 17 00:00:00 2001 From: odlomax Date: Wed, 2 Oct 2024 16:22:46 +0100 Subject: [PATCH 13/19] Cleaned up some garbage in test. --- src/tests/array/test_array_view_variant.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tests/array/test_array_view_variant.cc b/src/tests/array/test_array_view_variant.cc index d870ed092..e37754465 100644 --- a/src/tests/array/test_array_view_variant.cc +++ b/src/tests/array/test_array_view_variant.cc @@ -43,8 +43,6 @@ CASE("test variant assignment") { auto deviceView3 = make_device_view_variant(array3); auto deviceView4 = make_device_view_variant(arrayRef); - auto view = make_view(array1); - const auto visitVariants = [](auto& var1, auto& var2, auto var3, auto var4) { std::visit( [](auto&& view) { From 229b05b1aced403fadf34edddcc727fece86fa1a Mon Sep 17 00:00:00 2001 From: odlomax Date: Thu, 3 Oct 2024 08:42:53 +0100 Subject: [PATCH 14/19] Removed reference qualifier on visitor template parameter. --- src/tests/array/test_array_view_variant.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tests/array/test_array_view_variant.cc b/src/tests/array/test_array_view_variant.cc index e37754465..f6eaeeacf 100644 --- a/src/tests/array/test_array_view_variant.cc +++ b/src/tests/array/test_array_view_variant.cc @@ -45,7 +45,7 @@ CASE("test variant assignment") { const auto visitVariants = [](auto& var1, auto& var2, auto var3, auto var4) { std::visit( - [](auto&& view) { + [](auto view) { EXPECT((is_rank<1>(view))); EXPECT((is_value_type(view))); EXPECT((is_non_const_value_type(view))); @@ -53,7 +53,7 @@ CASE("test variant assignment") { var1); std::visit( - [](auto&& view) { + [](auto view) { EXPECT((is_rank<2>(view))); EXPECT((is_value_type(view))); EXPECT((is_non_const_value_type(view))); @@ -61,7 +61,7 @@ CASE("test variant assignment") { var2); std::visit( - [](auto&& view) { + [](auto view) { EXPECT((is_rank<3>(view))); EXPECT((is_value_type(view))); EXPECT((is_non_const_value_type(view))); @@ -69,7 +69,7 @@ CASE("test variant assignment") { var3); std::visit( - [](auto&& view) { + [](auto view) { EXPECT((is_rank<1>(view))); EXPECT((is_value_type(view))); EXPECT((is_non_const_value_type(view))); @@ -94,7 +94,7 @@ CASE("test std::visit") { auto rank1Tested = false; auto rank2Tested = false; - const auto visitor = [&](auto&& view) { + const auto visitor = [&](auto view) { if constexpr (is_rank<1>(view)) { EXPECT((is_value_type(view))); auto testValue = int{0}; From 7b9bdc923209fd981cd4164fd5d81ea72130f509 Mon Sep 17 00:00:00 2001 From: odlomax Date: Mon, 7 Oct 2024 16:32:34 +0100 Subject: [PATCH 15/19] Moved ValuesTypes and Ranks structs into array::detail namespace. --- src/atlas/array/ArrayViewVariant.h | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index 7bd33b15e..495c38cf6 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -47,19 +47,18 @@ struct VariantHelper, Ints, ArrayViews...> { template using Variant = typename VariantHelper::type; -} // namespace detail - -/// @brief Supported ArrayView value types. using ValueTypes = detail::Types; -/// @brief Supported ArrayView ranks. using Ranks = detail::Ints<1, 2, 3, 4, 5, 6, 7, 8, 9>; +} // namespace detail + /// @brief Variant containing all supported non-const ArrayView alternatives. -using ArrayViewVariant = detail::Variant; +using ArrayViewVariant = detail::Variant; /// @brief Variant containing all supported const ArrayView alternatives. -using ConstArrayViewVariant = detail::Variant; +using ConstArrayViewVariant = + detail::Variant; /// @brief Create an ArrayView and assign to an ArrayViewVariant. ArrayViewVariant make_view_variant(Array& array); From 152e8cf7c52ced7a016fd53041c167969c0f15ac Mon Sep 17 00:00:00 2001 From: odlomax Date: Mon, 7 Oct 2024 16:38:23 +0100 Subject: [PATCH 16/19] Tidied up naming consistency. --- src/atlas/array/ArrayViewVariant.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index 495c38cf6..4ed286edc 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -28,7 +28,7 @@ struct Types { template struct Ints {}; -template +template struct VariantHelper; // Recursively construct ArrayView std::variant from types Ts and ranks Is. From b901194cf40e45e0fa2a649b91b3e19b14820c70 Mon Sep 17 00:00:00 2001 From: odlomax Date: Mon, 7 Oct 2024 16:46:46 +0100 Subject: [PATCH 17/19] Renamed ValueType and Ranks structs. --- src/atlas/array/ArrayViewVariant.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index 4ed286edc..c78ee7dea 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -28,7 +28,8 @@ struct Types { template struct Ints {}; -template +template struct VariantHelper; // Recursively construct ArrayView std::variant from types Ts and ranks Is. @@ -44,21 +45,23 @@ struct VariantHelper, Ints, ArrayViews...> { using type = std::variant; }; -template -using Variant = typename VariantHelper::type; +template +using Variant = typename VariantHelper::type; -using ValueTypes = detail::Types; +using VariantValueTypes = + detail::Types; -using Ranks = detail::Ints<1, 2, 3, 4, 5, 6, 7, 8, 9>; +using VariantRanks = detail::Ints<1, 2, 3, 4, 5, 6, 7, 8, 9>; } // namespace detail /// @brief Variant containing all supported non-const ArrayView alternatives. -using ArrayViewVariant = detail::Variant; +using ArrayViewVariant = + detail::Variant; /// @brief Variant containing all supported const ArrayView alternatives. using ConstArrayViewVariant = - detail::Variant; + detail::Variant; /// @brief Create an ArrayView and assign to an ArrayViewVariant. ArrayViewVariant make_view_variant(Array& array); From 2ea78330ff2b49fe5a616f005b9a619ad9bb839f Mon Sep 17 00:00:00 2001 From: Oliver Lomax Date: Mon, 7 Oct 2024 16:55:29 +0100 Subject: [PATCH 18/19] Revert parameter names in ArrayViewVariant.h --- src/atlas/array/ArrayViewVariant.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index c78ee7dea..28a0d6e99 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -28,8 +28,7 @@ struct Types { template struct Ints {}; -template +template struct VariantHelper; // Recursively construct ArrayView std::variant from types Ts and ranks Is. From 2e1ac8e56b2c27e39495a5e9d47982f8f7e601af Mon Sep 17 00:00:00 2001 From: odlomax Date: Mon, 7 Oct 2024 16:58:02 +0100 Subject: [PATCH 19/19] Revert parameter names in ArrayViewVariant.h --- src/atlas/array/ArrayViewVariant.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/atlas/array/ArrayViewVariant.h b/src/atlas/array/ArrayViewVariant.h index 28a0d6e99..94dff8115 100644 --- a/src/atlas/array/ArrayViewVariant.h +++ b/src/atlas/array/ArrayViewVariant.h @@ -44,8 +44,8 @@ struct VariantHelper, Ints, ArrayViews...> { using type = std::variant; }; -template -using Variant = typename VariantHelper::type; +template +using Variant = typename VariantHelper::type; using VariantValueTypes = detail::Types;