From 5d7becd67a7918435bff6ac7bb964816153e8cb1 Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Tue, 6 Feb 2024 16:59:52 +0100 Subject: [PATCH 1/3] Fix test case not actually doing any testing Fixes #179 --- tests/unit/test_tensor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_tensor.cpp b/tests/unit/test_tensor.cpp index 539d57755..99f6a3790 100644 --- a/tests/unit/test_tensor.cpp +++ b/tests/unit/test_tensor.cpp @@ -97,7 +97,7 @@ TEST_CASE("Tensor", "[elements]") { auto t2 = Tensor(L"F", {L"i_2"}, {L"i_1"}); size_t t2_hash; REQUIRE_NOTHROW(t2_hash = hash_value(t2)); - REQUIRE_NOTHROW(t1_hash != t2_hash); + REQUIRE(t1_hash != t2_hash); } // SECTION("hash") From 9f871609e64f6559f5d103cda46641dccdfc2f8d Mon Sep 17 00:00:00 2001 From: Robert Adam Date: Thu, 8 Feb 2024 12:42:51 +0100 Subject: [PATCH 2/3] Fix broken lookup of hash_value function By bringing the Boost hash_value functions into the SeQuant namespace, some instantiations of the hash_value function resolved to the general Boost functions instead of using the specializations provided by SeQuant. In particular, expression objects with a hash_value member function would simply be hashed via a generic hash_range implementation (as every expression is also a range) instead of using the template specialization that delegates to the (memoizing) hash member function. This issue is resolved by not importing the functions from the Boost namespace but instead keeping two specializations in the SeQuant namespace that either resolve to the member function (if present) or the generic Boost function (if the object does not have a hash member function and a suitable overload on Boost's side is found). --- SeQuant/core/hash.hpp | 64 ++++++++++++++++------------------------ tests/unit/test_mbpt.cpp | 6 ++-- 2 files changed, 28 insertions(+), 42 deletions(-) diff --git a/SeQuant/core/hash.hpp b/SeQuant/core/hash.hpp index 59851f2a9..1cc371295 100644 --- a/SeQuant/core/hash.hpp +++ b/SeQuant/core/hash.hpp @@ -39,8 +39,6 @@ constexpr hash::Impl hash_version() { #endif } -class Expr; - namespace detail { template struct has_hash_value_member_fn_helper : public std::false_type {}; @@ -54,30 +52,15 @@ template static constexpr bool has_hash_value_member_fn_v = detail::has_hash_value_member_fn_helper::value; -#ifdef SEQUANT_USE_SYSTEM_BOOST_HASH -#if SEQUANT_BOOST_VERSION < 108100 -template >> -auto hash_value(const T& obj) { - return obj.hash_value(); -} -#endif -#endif // SEQUANT_USE_SYSTEM_BOOST_HASH - namespace detail { template > struct has_boost_hash_value : std::false_type {}; -#ifdef SEQUANT_USE_SYSTEM_BOOST_HASH template struct has_boost_hash_value< T, std::void_t()))>> : std::true_type {}; -#endif // SEQUANT_USE_SYSTEM_BOOST_HASH - -template -constexpr bool has_boost_hash_value_v = has_boost_hash_value::value; template > struct has_hash_value : std::false_type {}; @@ -87,12 +70,30 @@ struct has_hash_value< T, std::void_t()))>> : std::true_type {}; +} // namespace detail + template -constexpr bool has_hash_value_v = has_hash_value::value; +constexpr bool has_boost_hash_value_v = + detail::has_boost_hash_value::value; -} // namespace detail +template +constexpr bool has_hash_value_v = detail::has_hash_value::value; + +// hash_value specialization for types that have a hash_value member function +template , short> = 0> +auto hash_value(const T& obj) { + return obj.hash_value(); +} -using sequant_boost::hash_value; +// hash_value specialization that don't have a hash_value member function but +// have an applicable boost::hash_value function +template && + has_boost_hash_value_v, + int> = 0> +auto hash_value(const T& obj) { + return sequant_boost::hash_value(obj); +} // clang-format off // rationale: @@ -187,13 +188,7 @@ inline void range(std::size_t& seed, It first, It last) { template std::size_t hash_range(It begin, It end) { if (begin != end) { - std::size_t seed; - if constexpr (has_hash_value_member_fn_v>) - seed = begin->hash_value(); - else { - using sequant_boost::hash_value; - [[maybe_unused]] std::size_t seed = hash_value(*begin); - } + std::size_t seed = hash_value(*begin); sequant_boost::hash_range(seed, begin + 1, end); return seed; } else @@ -207,22 +202,13 @@ void hash_range(size_t& seed, It begin, It end) { } template -struct _< - T, std::enable_if_t)&&meta::is_range_v>> { +struct _)&&meta::is_range_v>> { std::size_t operator()(T const& v) const { return range(begin(v), end(v)); } }; template -struct _)&&meta::is_range_v)>> { - std::size_t operator()(T const& v) const { - if constexpr (has_hash_value_member_fn_v) - return v.hash_value(); - else { - using sequant_boost::hash_value; - return hash_value(v); - } - } +struct _)&&meta::is_range_v)>> { + std::size_t operator()(T const& v) const { return hash_value(v); } }; template diff --git a/tests/unit/test_mbpt.cpp b/tests/unit/test_mbpt.cpp index 269a112f6..3d1954fff 100644 --- a/tests/unit/test_mbpt.cpp +++ b/tests/unit/test_mbpt.cpp @@ -279,7 +279,7 @@ TEST_CASE("NBodyOp", "[mbpt]") { if constexpr (hash_version() == hash::Impl::BoostPre181) { REQUIRE( to_latex(simplify(f * t * t)) == - to_latex(ex(2) * f * t1 * t2 + f * t1 * t1 + f * t2 * t2)); + to_latex(f * t1 * t1 + f * t2 * t2 + ex(2) * f * t1 * t2)); } else { // std::wcout << "to_latex(simplify(f * t * t)): " // << to_latex(simplify(f * t * t)) << std::endl; @@ -290,8 +290,8 @@ TEST_CASE("NBodyOp", "[mbpt]") { if constexpr (hash_version() == hash::Impl::BoostPre181) { REQUIRE(to_latex(simplify(f * t * t * t)) == - to_latex(f * t1 * t1 * t1 + ex(3) * f * t1 * t2 * t2 + - f * t2 * t2 * t2 + ex(3) * f * t1 * t1 * t2)); + to_latex(ex(3) * f * t1 * t2 * t2 + f * t2 * t2 * t2 + + ex(3) * f * t1 * t1 * t2 + f * t1 * t1 * t1)); } else { // std::wcout << "to_latex(simplify(f * t * t * t): " // << to_latex(simplify(f * t * t * t)) << std::endl; From 074dc6a1e99fd5e99dda6e3ed26b7920daf0b41c Mon Sep 17 00:00:00 2001 From: Eduard Valeyev Date: Wed, 14 Feb 2024 16:56:21 -0500 Subject: [PATCH 3/3] assume boost::hash_value takes const T& --- SeQuant/core/hash.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SeQuant/core/hash.hpp b/SeQuant/core/hash.hpp index 1cc371295..f71cea284 100644 --- a/SeQuant/core/hash.hpp +++ b/SeQuant/core/hash.hpp @@ -58,8 +58,8 @@ template > struct has_boost_hash_value : std::false_type {}; template -struct has_boost_hash_value< - T, std::void_t()))>> +struct has_boost_hash_value()))>> : std::true_type {}; template >