From d5823f46cc36584b97879b16344489974ce94cd2 Mon Sep 17 00:00:00 2001 From: DerThorsten Date: Thu, 3 Jun 2021 11:31:40 +0200 Subject: [PATCH 1/3] Initial draft for a specialized computed_assign for xfixed * added overload for fixed/nonfixed shape via enable_if * added tests --- include/xtensor/xassign.hpp | 66 +++++++++++++++++++++++++- test/test_xassign.cpp | 93 +++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 2 deletions(-) diff --git a/include/xtensor/xassign.hpp b/include/xtensor/xassign.hpp index 314ae6bd2..c4a699ef1 100644 --- a/include/xtensor/xassign.hpp +++ b/include/xtensor/xassign.hpp @@ -74,6 +74,30 @@ namespace xt static void assign_data(xexpression& e1, const xexpression& e2, bool trivial); }; + namespace detail{ + + template + struct is_fixed_shape : public std::false_type + { + }; + + template + struct is_fixed_shape> : public std::true_type + { + }; + + template + using has_fixed_shape = is_fixed_shape; + + template + using enable_if_has_fixed_shape_t = std::enable_if_t::value>; + + template + using enable_if_has_no_fixed_shape_t = std::enable_if_t::value>; + + } + + template class xexpression_assigner : public xexpression_assigner_base { @@ -84,7 +108,12 @@ namespace xt template static void assign_xexpression(E1& e1, const E2& e2); - template + + + template * = nullptr> + static void computed_assign(xexpression& e1, const xexpression& e2); + + template * = nullptr> static void computed_assign(xexpression& e1, const xexpression& e2); template @@ -415,7 +444,7 @@ namespace xt } template - template + template *> inline void xexpression_assigner::computed_assign(xexpression& e1, const xexpression& e2) { using shape_type = typename E1::shape_type; @@ -444,6 +473,39 @@ namespace xt } } + + template + template *> + inline void xexpression_assigner::computed_assign(xexpression& e1, const xexpression& e2) + { + using shape_type = typename E1::shape_type; + + E1& de1 = e1.derived_cast(); + const E2& de2 = e2.derived_cast(); + + if(de1.dimension() != de2.dimension()) + { + // not sure if best error + throw_broadcast_error(de1.shape(), de2.shape()); + } + else + { + auto && shape1 = de1.shape(); + auto && shape2 = de2.shape(); + if(std::equal(shape1.begin(), shape1.end(), shape2.begin())) + { + // the tests fail if I just set it to true for the case + // when creating e2 itself involved broadcasting + base_type::assign_data(e1, e2, false/*trivial_broadcast*/); + } + else + { + // not sure if best error + throw_broadcast_error(de1.shape(), de2.shape()); + } + } + } + template template inline void xexpression_assigner::scalar_computed_assign(xexpression& e1, const E2& e2, F&& f) diff --git a/test/test_xassign.cpp b/test/test_xassign.cpp index 3f5618a66..f61bac185 100644 --- a/test/test_xassign.cpp +++ b/test/test_xassign.cpp @@ -10,6 +10,7 @@ #include "gtest/gtest.h" #include "xtensor/xarray.hpp" #include "xtensor/xtensor.hpp" +#include "xtensor/xfixed.hpp" #include "xtensor/xassign.hpp" #include "xtensor/xnoalias.hpp" @@ -143,6 +144,98 @@ namespace xt EXPECT_EQ(a.shape(0), 2); EXPECT_EQ(a.shape(1), 3); } + } + + TEST(xassign, fixed_shape) + { + // matching shape 1D + { + xt::xtensor_fixed> a = {2,3}; + xt::xtensor_fixed> b = {3,4}; + + xt::noalias(a) += b; + + EXPECT_EQ(a(0), 5); + EXPECT_EQ(a(1), 7); + } + //matching shape 2D + { + xt::xtensor_fixed> aa = {{1,2},{3,4}}; + xt::xtensor_fixed> a(aa); + xt::xtensor_fixed> b = {{3,4},{5,6}}; + xt::noalias(a) += b; + EXPECT_EQ(a(0,0), aa(0,0) + b(0,0)); + EXPECT_EQ(a(0,1), aa(0,1) + b(0,1)); + EXPECT_EQ(a(1,0), aa(1,0) + b(1,0)); + EXPECT_EQ(a(1,1), aa(1,1) + b(1,1)); + } + // b is broadcasted with matching dimension (first axis is singleton) + { + xt::xtensor_fixed> aa = {{1,2},{3,4}}; + xt::xtensor_fixed> a(aa); + xt::xtensor_fixed> b = {{5,6}}; + EXPECT_EQ(b.shape(0),2); + EXPECT_EQ(b.shape(1),1); + xt::noalias(a) += b; + + EXPECT_EQ(a(0,0), aa(0,0) + b.at(0,0)); + EXPECT_EQ(a(0,1), aa(0,1) + b.at(0,0)); + EXPECT_EQ(a(1,0), aa(1,0) + b.at(1,0)); + EXPECT_EQ(a(1,1), aa(1,1) + b.at(1,0)); + } + // b is broadcasted with matching dimension (second axis is singleton) + { + xt::xtensor_fixed> aa = {{1,2},{3,4}}; + xt::xtensor_fixed> a(aa); + xt::xtensor_fixed> b = {{3,4}}; + EXPECT_EQ(b.shape(0),1); + EXPECT_EQ(b.shape(1),2); + xt::noalias(a) += b; + + EXPECT_EQ(a(0,0), aa(0,0) + b(0,0)); + EXPECT_EQ(a(0,1), aa(0,1) + b(0,1)); + EXPECT_EQ(a(1,0), aa(1,0) + b(0,0)); + EXPECT_EQ(a(1,1), aa(1,1) + b(0,1)); + } + // broadcast with non matching dimensions + { + xt::xtensor_fixed> aa = {{1,2},{3,4}}; + xt::xtensor_fixed> a(aa); + xt::xtensor_fixed> b = {3,4}; + + xt::noalias(a) += b; + + EXPECT_EQ(a(0,0), aa(0,0) + b(0)); + EXPECT_EQ(a(0,1), aa(0,1) + b(1)); + EXPECT_EQ(a(1,0), aa(1,0) + b(0)); + EXPECT_EQ(a(1,1), aa(1,1) + b(1)); + } + } + TEST(xassign, fixed_raises) + { + // cannot broadcast a itself on assignment + { + xt::xtensor_fixed> a = {2,3}; + xt::xtensor_fixed> b = {{3,4},{3,4}}; + + EXPECT_THROW(xt::noalias(a) += b, xt::broadcast_error); + } + + // cannot broadcast a itself on assignment + { + xt::xtensor_fixed> a = {{3,4}}; + xt::xtensor_fixed> b = {{3,4},{3,4}}; + + EXPECT_THROW(xt::noalias(a) += b, xt::broadcast_error); + } + + // cannot broadcast a itself on assignment + { + xt::xtensor_fixed> a = {{3},{4}}; + xt::xtensor_fixed> b = {{3,4},{3,4}}; + + EXPECT_THROW(xt::noalias(a) += b, xt::broadcast_error); + } } } From 923f91a99bd35b3f981afa045405f33b86fe7dea Mon Sep 17 00:00:00 2001 From: DerThorsten Date: Thu, 3 Jun 2021 12:37:07 +0200 Subject: [PATCH 2/3] changed tests --- include/xtensor/xassign.hpp | 2 +- test/test_xassign.cpp | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/include/xtensor/xassign.hpp b/include/xtensor/xassign.hpp index c4a699ef1..9e701bc1c 100644 --- a/include/xtensor/xassign.hpp +++ b/include/xtensor/xassign.hpp @@ -494,7 +494,7 @@ namespace xt auto && shape2 = de2.shape(); if(std::equal(shape1.begin(), shape1.end(), shape2.begin())) { - // the tests fail if I just set it to true for the case + // the tests fail if just set it to true for the case // when creating e2 itself involved broadcasting base_type::assign_data(e1, e2, false/*trivial_broadcast*/); } diff --git a/test/test_xassign.cpp b/test/test_xassign.cpp index f61bac185..d2f22ead5 100644 --- a/test/test_xassign.cpp +++ b/test/test_xassign.cpp @@ -174,7 +174,21 @@ namespace xt { xt::xtensor_fixed> aa = {{1,2},{3,4}}; xt::xtensor_fixed> a(aa); - xt::xtensor_fixed> b = {{5,6}}; + xt::xarray b = {{5,6}}; + EXPECT_EQ(b.shape(0),1); + EXPECT_EQ(b.shape(1),2); + xt::noalias(a) += b; + + EXPECT_EQ(a(0,0), aa(0,0) + b(0,0)); + EXPECT_EQ(a(0,1), aa(0,1) + b(0,1)); + EXPECT_EQ(a(1,0), aa(1,0) + b(0,0)); + EXPECT_EQ(a(1,1), aa(1,1) + b(0,1)); + } + // b is broadcasted with matching dimension (second axis is singleton) + { + xt::xtensor_fixed> aa = {{1,2},{3,4}}; + xt::xtensor_fixed> a(aa); + xt::xtensor_fixed> b = {{5},{6}}; EXPECT_EQ(b.shape(0),2); EXPECT_EQ(b.shape(1),1); xt::noalias(a) += b; @@ -184,7 +198,7 @@ namespace xt EXPECT_EQ(a(1,0), aa(1,0) + b.at(1,0)); EXPECT_EQ(a(1,1), aa(1,1) + b.at(1,0)); } - // b is broadcasted with matching dimension (second axis is singleton) + // b is broadcasted with matching dimension (first axis is singleton) { xt::xtensor_fixed> aa = {{1,2},{3,4}}; xt::xtensor_fixed> a(aa); From ff611e396a04f76cbc8f1f21cec246e8686e98cb Mon Sep 17 00:00:00 2001 From: DerThorsten Date: Thu, 3 Jun 2021 13:15:35 +0200 Subject: [PATCH 3/3] skip fixed part of test for vs --- test/test_xassign.cpp | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/test/test_xassign.cpp b/test/test_xassign.cpp index d2f22ead5..2a4e4caab 100644 --- a/test/test_xassign.cpp +++ b/test/test_xassign.cpp @@ -7,11 +7,19 @@ * The full license is in the file LICENSE, distributed with this software. * ****************************************************************************/ + + +#if defined(_MSC_VER) && !defined(__clang__) +#define VS_SKIP_XFIXED 1 +#endif + + #include "gtest/gtest.h" #include "xtensor/xarray.hpp" #include "xtensor/xtensor.hpp" +#ifndef VS_SKIP_XFIXED #include "xtensor/xfixed.hpp" - +#endif #include "xtensor/xassign.hpp" #include "xtensor/xnoalias.hpp" #include "test_common.hpp" @@ -20,6 +28,19 @@ #include + +// On VS2015, when compiling in x86 mode, alignas(T) leads to C2718 +// when used for a function parameter, even indirectly. This means that +// we cannot pass parameters whose class is declared with alignas specifier +// or any type wrapping or inheriting from such a type. +// The xtensor_fixed class internally uses aligned_array which is declared as +// alignas(something_different_from_0), hence the workaround. +#if _MSC_VER < 1910 && !_WIN64 +#define VS_X86_WORKAROUND 1 +#endif + + + // a dummy shape *not derived* from std::vector but compatible template class my_vector @@ -146,6 +167,13 @@ namespace xt } } + // test_fixed removed from MSVC x86 because of recurring ICE. + // Will be enabled again when the compiler is fixed + + #ifndef VS_SKIP_XFIXED + #if (_MSC_VER < 1910 && _WIN64) || (_MSC_VER >= 1910 && !defined(DISABLE_VS2017)) || !defined(_MSC_VER) + + TEST(xassign, fixed_shape) { // matching shape 1D @@ -252,4 +280,7 @@ namespace xt EXPECT_THROW(xt::noalias(a) += b, xt::broadcast_error); } } + #endif + #endif // VS_SKIP_XFIXED + }