From f544ccf3085b6ca33320961d78177f38836e45e9 Mon Sep 17 00:00:00 2001 From: Markus Blatt Date: Wed, 28 Jan 2015 21:55:35 +0100 Subject: [PATCH 1/2] Constified the compute reduction functions. These should and have to be used with a const object. --- opm/core/linalg/ParallelIstlInformation.hpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/opm/core/linalg/ParallelIstlInformation.hpp b/opm/core/linalg/ParallelIstlInformation.hpp index 9c60de9f..db43f421 100644 --- a/opm/core/linalg/ParallelIstlInformation.hpp +++ b/opm/core/linalg/ParallelIstlInformation.hpp @@ -142,7 +142,7 @@ public: communicator.free(); } template - void updateOwnerMask(const T& container) + void updateOwnerMask(const T& container) const { if( ! indexSet_ ) { @@ -180,7 +180,7 @@ public: /// \param value The initial value or a tuple of them. template void computeReduction(const Container& container, BinaryOperator binaryOperator, - T& value) + T& value) const { computeReduction(container, binaryOperator, value, is_tuple()); } @@ -190,7 +190,7 @@ private: /// This is a helper function to prepare for calling computeTupleReduction. template void computeReduction(const Container& container, BinaryOperator binaryOperator, - T& value, std::integral_constant) + T& value, std::integral_constant) const { computeTupleReduction(container, binaryOperator, value); } @@ -199,7 +199,7 @@ private: /// This is a helper function to prepare for calling computeTupleReduction. template void computeReduction(const Container& container, BinaryOperator binaryOperator, - T& value, std::integral_constant) + T& value, std::integral_constant) const { std::tuple containers=std::tuple(container); auto values=std::make_tuple(value); @@ -211,7 +211,7 @@ private: template void computeTupleReduction(const std::tuple& containers, std::tuple& operators, - std::tuple& values) + std::tuple& values) const { static_assert(std::tuple_size >::value== std::tuple_size >::value, @@ -243,14 +243,14 @@ private: typename std::enable_if::type computeGlobalReduction(const std::tuple&, std::tuple&, - std::tuple&) + std::tuple&) const {} /// \brief TMP for computing the the global reduction after receiving the local ones. template typename std::enable_if::type computeGlobalReduction(const std::tuple& receivedValues, std::tuple& operators, - std::tuple& values) + std::tuple& values) const { auto& val=std::get(values); val = std::get(operators).localOperator()(val, std::get(receivedValues)); @@ -263,14 +263,14 @@ private: typename std::enable_if::type computeLocalReduction(const std::tuple&, std::tuple&, - std::tuple&) + std::tuple&) const {} /// \brief TMP for computing the the local reduction on the DOF that the process owns. template typename std::enable_if::type computeLocalReduction(const std::tuple& containers, std::tuple& operators, - std::tuple& values) + std::tuple& values) const { const auto& container = std::get(containers); if( container.size() ) From 086c405e9d6dff1ce4ec7b48e6c2ede58073caf5 Mon Sep 17 00:00:00 2001 From: Markus Blatt Date: Wed, 28 Jan 2015 21:56:39 +0100 Subject: [PATCH 2/2] Do not rely on begin()/end() of the containers to compute reductions. One would think that such an assumption is safe in any case, wouldn't one? But foen Eigen'S container this does not hold. They do not provide STL compliant iterators, and access to them. With this patch make the even stricter assumption that the containers are random access and use operator[] instead of iterators. --- opm/core/linalg/ParallelIstlInformation.hpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/opm/core/linalg/ParallelIstlInformation.hpp b/opm/core/linalg/ParallelIstlInformation.hpp index db43f421..09a00d16 100644 --- a/opm/core/linalg/ParallelIstlInformation.hpp +++ b/opm/core/linalg/ParallelIstlInformation.hpp @@ -276,17 +276,22 @@ private: if( container.size() ) { auto& reduceOperator = std::get(operators); - auto newVal = container.begin(); + // Eigen:Block does not support STL iterators!!!! + // Therefore we need to rely on the harder random-access + // property of the containers. But this should be save, too. + // Just commenting out code in the hope that Eigen might improve + // in this regard in the future. + //auto newVal = container.begin(); auto mask = ownerMask_.begin(); auto& value = std::get(values); - value = reduceOperator.maskValue(*newVal, *mask); + value = reduceOperator.maskValue(container[0], *mask); ++mask; - ++newVal; + //++newVal; - for( auto endVal=container.end(); newVal!=endVal; - ++newVal, ++mask ) + for( auto endVal=ownerMask_.end(); mask!=endVal; + /*++newVal,*/ ++mask ) { - value = reduceOperator(value, *newVal, *mask); + value = reduceOperator(value, container[mask-ownerMask_.begin()], *mask); } } computeLocalReduction(containers, operators, values);