diff --git a/benchmark/histogram_parallel_filling.cpp b/benchmark/histogram_parallel_filling.cpp index c58c8ac8..dfce607e 100644 --- a/benchmark/histogram_parallel_filling.cpp +++ b/benchmark/histogram_parallel_filling.cpp @@ -5,7 +5,7 @@ // or copy at http://www.boost.org/LICENSE_1_0.txt) #include -#include +#include #include #include #include @@ -29,7 +29,7 @@ using namespace boost::histogram; using namespace std::chrono_literals; using DS = dense_storage; -using DSTS = dense_storage>; +using DSTS = dense_storage>; static void NoThreads(benchmark::State& state) { std::default_random_engine gen(1); diff --git a/include/boost/histogram/accumulators/mean.hpp b/include/boost/histogram/accumulators/mean.hpp index 36555039..f6f15d2a 100644 --- a/include/boost/histogram/accumulators/mean.hpp +++ b/include/boost/histogram/accumulators/mean.hpp @@ -30,36 +30,41 @@ class mean { using value_type = ValueType; using const_reference = const value_type&; + struct data_type { + value_type sum_; + value_type mean_; + value_type sum_of_deltas_squared_; + }; + mean() = default; /// Allow implicit conversion from mean. template - mean(const mean& o) noexcept - : sum_{o.sum_}, mean_{o.mean_}, sum_of_deltas_squared_{o.sum_of_deltas_squared_} {} + mean(const mean& o) noexcept : data_{o.data_} {} /// Initialize to external count, mean, and variance. mean(const_reference n, const_reference mean, const_reference variance) noexcept - : sum_(n), mean_(mean), sum_of_deltas_squared_(variance * (n - 1)) {} + : data_{n, mean, variance * (n - 1)} {} /// Insert sample x. void operator()(const_reference x) noexcept { - sum_ += static_cast(1); - const auto delta = x - mean_; - mean_ += delta / sum_; - sum_of_deltas_squared_ += delta * (x - mean_); + data_.sum_ += static_cast(1); + const auto delta = x - data_.mean_; + data_.mean_ += delta / data_.sum_; + data_.sum_of_deltas_squared_ += delta * (x - data_.mean_); } /// Insert sample x with weight w. void operator()(const weight_type& w, const_reference x) noexcept { - sum_ += w.value; - const auto delta = x - mean_; - mean_ += w.value * delta / sum_; - sum_of_deltas_squared_ += w.value * delta * (x - mean_); + data_.sum_ += w.value; + const auto delta = x - data_.mean_; + data_.mean_ += w.value * delta / data_.sum_; + data_.sum_of_deltas_squared_ += w.value * delta * (x - data_.mean_); } /// Add another mean accumulator. mean& operator+=(const mean& rhs) noexcept { - if (rhs.sum_ == 0) return *this; + if (rhs.data_.sum_ == 0) return *this; /* sum_of_deltas_squared @@ -75,20 +80,20 @@ class mean { Putting it together: sum_of_deltas_squared - = sum_of_deltas_squared_1 + n1 (mu1 - mu))^2 - + sum_of_deltas_squared_2 + n2 (mu2 - mu))^2 + = sum_of_deltas_squared_1 + n1 (mu - mu1))^2 + + sum_of_deltas_squared_2 + n2 (mu - mu2))^2 */ - const auto n1 = sum_; - const auto mu1 = mean_; - const auto n2 = rhs.sum_; - const auto mu2 = rhs.mean_; + const auto n1 = data_.sum_; + const auto mu1 = data_.mean_; + const auto n2 = rhs.data_.sum_; + const auto mu2 = rhs.data_.mean_; - sum_ += rhs.sum_; - mean_ = (n1 * mu1 + n2 * mu2) / sum_; - sum_of_deltas_squared_ += rhs.sum_of_deltas_squared_; - sum_of_deltas_squared_ += n1 * detail::square(mean_ - mu1); - sum_of_deltas_squared_ += n2 * detail::square(mean_ - mu2); + data_.sum_ += rhs.data_.sum_; + data_.mean_ = (n1 * mu1 + n2 * mu2) / data_.sum_; + data_.sum_of_deltas_squared_ += rhs.data_.sum_of_deltas_squared_; + data_.sum_of_deltas_squared_ += n1 * detail::square(data_.mean_ - mu1); + data_.sum_of_deltas_squared_ += n2 * detail::square(data_.mean_ - mu2); return *this; } @@ -98,14 +103,14 @@ class mean { This acts as if all samples were scaled by the value. */ mean& operator*=(const_reference s) noexcept { - mean_ *= s; - sum_of_deltas_squared_ *= s * s; + data_.mean_ *= s; + data_.sum_of_deltas_squared_ *= s * s; return *this; } bool operator==(const mean& rhs) const noexcept { - return sum_ == rhs.sum_ && mean_ == rhs.mean_ && - sum_of_deltas_squared_ == rhs.sum_of_deltas_squared_; + return data_.sum_ == rhs.data_.sum_ && data_.mean_ == rhs.data_.mean_ && + data_.sum_of_deltas_squared_ == rhs.data_.sum_of_deltas_squared_; } bool operator!=(const mean& rhs) const noexcept { return !operator==(rhs); } @@ -116,19 +121,21 @@ class mean { see documentation of value() and variance(). count() can be used to compute the variance of the mean by dividing variance() by count(). */ - const_reference count() const noexcept { return sum_; } + const_reference count() const noexcept { return data_.sum_; } /** Return mean value of accumulated samples. The result is undefined, if `count() < 1`. */ - const_reference value() const noexcept { return mean_; } + const_reference value() const noexcept { return data_.mean_; } /** Return variance of accumulated samples. The result is undefined, if `count() < 2`. */ - value_type variance() const noexcept { return sum_of_deltas_squared_ / (sum_ - 1); } + value_type variance() const noexcept { + return data_.sum_of_deltas_squared_ / (data_.sum_ - 1); + } template void serialize(Archive& ar, unsigned version) { @@ -136,18 +143,18 @@ class mean { // read only std::size_t sum; ar& make_nvp("sum", sum); - sum_ = static_cast(sum); + data_.sum_ = static_cast(sum); } else { - ar& make_nvp("sum", sum_); + ar& make_nvp("sum", data_.sum_); } - ar& make_nvp("mean", mean_); - ar& make_nvp("sum_of_deltas_squared", sum_of_deltas_squared_); + ar& make_nvp("mean", data_.mean_); + ar& make_nvp("sum_of_deltas_squared", data_.sum_of_deltas_squared_); } private: - value_type sum_{}; - value_type mean_{}; - value_type sum_of_deltas_squared_{}; + data_type data_{0, 0, 0}; + + friend struct ::boost::histogram::unsafe_access; }; } // namespace accumulators diff --git a/include/boost/histogram/accumulators/ostream.hpp b/include/boost/histogram/accumulators/ostream.hpp index 7519bf0e..586d2402 100644 --- a/include/boost/histogram/accumulators/ostream.hpp +++ b/include/boost/histogram/accumulators/ostream.hpp @@ -63,7 +63,8 @@ std::basic_ostream& operator<<(std::basic_ostream& template std::basic_ostream& operator<<(std::basic_ostream& os, const sum& x) { - if (os.width() == 0) return os << "sum(" << x.large() << " + " << x.small() << ")"; + if (os.width() == 0) + return os << "sum(" << x.large_part() << " + " << x.small_part() << ")"; return detail::handle_nonzero_width(os, x); } diff --git a/include/boost/histogram/accumulators/sum.hpp b/include/boost/histogram/accumulators/sum.hpp index eefdff13..1a7d8442 100644 --- a/include/boost/histogram/accumulators/sum.hpp +++ b/include/boost/histogram/accumulators/sum.hpp @@ -43,11 +43,11 @@ class sum { /// Allow implicit conversion from sum template - sum(const sum& s) noexcept : sum(s.large(), s.small()) {} + sum(const sum& s) noexcept : sum(s.large_part(), s.small_part()) {} /// Initialize sum explicitly with large and small parts - sum(const_reference large, const_reference small) noexcept - : large_(large), small_(small) {} + sum(const_reference large_part, const_reference small_part) noexcept + : large_(large_part), small_(small_part) {} /// Increment sum by one sum& operator++() noexcept { return operator+=(1); } @@ -96,10 +96,10 @@ class sum { value_type value() const noexcept { return large_ + small_; } /// Return large part of the sum. - const_reference large() const noexcept { return large_; } + const_reference large_part() const noexcept { return large_; } /// Return small part of the sum. - const_reference small() const noexcept { return small_; } + const_reference small_part() const noexcept { return small_; } // lossy conversion to value type must be explicit explicit operator value_type() const noexcept { return value(); } @@ -156,6 +156,25 @@ class sum { // end: extra operators + // windows.h illegially uses `#define small char` which breaks this now deprecated API +#if !defined(small) + + /// Return large part of the sum. + [[deprecated("use large_part() instead; " + "large() will be removed in boost-1.80")]] const_reference + large() const noexcept { + return large_; + } + + /// Return small part of the sum. + [[deprecated("use small_part() instead; " + "small() will be removed in boost-1.80")]] const_reference + small() const noexcept { + return small_; + } + +#endif + private: value_type large_{}; value_type small_{}; diff --git a/include/boost/histogram/algorithm/reduce.hpp b/include/boost/histogram/algorithm/reduce.hpp index 0646db34..b0f7c7fc 100644 --- a/include/boost/histogram/algorithm/reduce.hpp +++ b/include/boost/histogram/algorithm/reduce.hpp @@ -33,7 +33,8 @@ namespace algorithm { */ using reduce_command = detail::reduce_command; -using reduce_option [[deprecated("use reduce_command instead")]] = +using reduce_option [[deprecated("use reduce_command instead; " + "reduce_option will be removed in boost-1.80")]] = reduce_command; ///< deprecated /** Shrink command to be used in `reduce`. diff --git a/include/boost/histogram/axis/traits.hpp b/include/boost/histogram/axis/traits.hpp index 934969f7..8e75918f 100644 --- a/include/boost/histogram/axis/traits.hpp +++ b/include/boost/histogram/axis/traits.hpp @@ -194,7 +194,9 @@ template using get_options = decltype(detail::traits_options(detail::priority<2>{})); template -using static_options [[deprecated("use get_options instead")]] = get_options; +using static_options [[deprecated("use get_options instead; " + "static_options will be removed in boost-1.80")]] = + get_options; #else struct get_options; @@ -223,7 +225,10 @@ template using is_inclusive = decltype(detail::traits_is_inclusive(detail::priority<1>{})); template -using static_is_inclusive [[deprecated("use is_inclusive instead")]] = is_inclusive; +using static_is_inclusive + [[deprecated("use is_inclusive instead; " + "static_is_inclusive will be removed in boost-1.80")]] = + is_inclusive; #else struct is_inclusive; diff --git a/include/boost/histogram/detail/fill_n.hpp b/include/boost/histogram/detail/fill_n.hpp index 128bc4c1..520b301e 100644 --- a/include/boost/histogram/detail/fill_n.hpp +++ b/include/boost/histogram/detail/fill_n.hpp @@ -242,6 +242,7 @@ void fill_n_1(const std::size_t offset, S& storage, A& axes, const std::size_t v for_each_axis(axes, [&](const auto& ax) { all_inclusive &= axis::traits::inclusive(ax); }); if (axes_rank(axes) == 1) { + // Optimization: benchmark shows that this makes filling dynamic 1D histogram faster axis::visit( [&](auto& ax) { std::tuple axes{ax}; diff --git a/include/boost/histogram/indexed.hpp b/include/boost/histogram/indexed.hpp index 47507f0b..3c9e8490 100644 --- a/include/boost/histogram/indexed.hpp +++ b/include/boost/histogram/indexed.hpp @@ -64,7 +64,9 @@ class BOOST_ATTRIBUTE_NODISCARD indexed_range { using value_type = typename std::iterator_traits::value_type; class iterator; - using range_iterator [[deprecated("use iterator instead")]] = iterator; ///< deprecated + using range_iterator [[deprecated("use iterator instead; " + "range_iterator will be removed in boost-1.80")]] = + iterator; ///< deprecated /** Lightweight view to access value and index of current cell. @@ -84,7 +86,8 @@ class BOOST_ATTRIBUTE_NODISCARD indexed_range { public: using const_reference = const axis::index_type&; - using reference [[deprecated("use const_reference instead")]] = + using reference [[deprecated("use const_reference instead; " + "reference will be removed in boost-1.80")]] = const_reference; ///< deprecated /// implementation detail diff --git a/include/boost/histogram/unsafe_access.hpp b/include/boost/histogram/unsafe_access.hpp index 1fff4c0f..4783694d 100644 --- a/include/boost/histogram/unsafe_access.hpp +++ b/include/boost/histogram/unsafe_access.hpp @@ -97,8 +97,8 @@ struct unsafe_access { Get buffer of unlimited_storage. @param storage instance of unlimited_storage. */ - template - static constexpr auto& unlimited_storage_buffer(unlimited_storage& storage) { + template + static constexpr auto& unlimited_storage_buffer(T& storage) { return storage.buffer_; } @@ -107,8 +107,17 @@ struct unsafe_access { @param storage instance of storage_adaptor. */ template - static constexpr auto& storage_adaptor_impl(storage_adaptor& storage) { - return static_cast::impl_type&>(storage); + static constexpr auto& storage_adaptor_impl(T& storage) { + return static_cast(storage); + } + + /** + Get internal data of accumulator. + @param obj instance of accumulator. + */ + template + static constexpr auto& accumulator_data(T& m) { + return m.data_; } }; diff --git a/test/Jamfile b/test/Jamfile index 33b50a98..a61e16ce 100644 --- a/test/Jamfile +++ b/test/Jamfile @@ -157,11 +157,14 @@ alias libserial : # for builds without optional boost dependencies alias minimal : cxx14 cxx17 failure threading ; +# for builds with optional boost dependencies +alias optional_boost : accumulators range units serialization ; + # all tests -alias all : minimal not_windows odr accumulators range units serialization ; +alias all : minimal not_windows odr optional_boost ; # all except "failure", because it is distracting during development -alias develop : cxx14 cxx17 threading not_windows odr accumulators range units serialization ; +alias develop : odr cxx14 cxx17 threading not_windows optional_boost ; explicit minimal ; explicit all ; @@ -176,3 +179,4 @@ explicit range ; explicit units ; explicit serialization ; explicit libserial ; +explicit optional_boost ; diff --git a/test/accumulators_mean_test.cpp b/test/accumulators_mean_test.cpp index f7fe938a..46cd4759 100644 --- a/test/accumulators_mean_test.cpp +++ b/test/accumulators_mean_test.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include "is_close.hpp" @@ -110,5 +111,16 @@ int main() { BOOST_TEST_IS_CLOSE(a.variance(), b.variance(), 1e-3); } + // unsafe_access + { + m_t a; + a(1); + a(2); + + BOOST_TEST_EQ(a.count(), 2); + unsafe_access::accumulator_data(a).sum_ = 1; + BOOST_TEST_EQ(a.count(), 1); + } + return boost::report_errors(); } diff --git a/test/accumulators_sum_test.cpp b/test/accumulators_sum_test.cpp index 31463e7d..b5708646 100644 --- a/test/accumulators_sum_test.cpp +++ b/test/accumulators_sum_test.cpp @@ -26,12 +26,19 @@ int main() { ++sum; BOOST_TEST_EQ(sum, 1); BOOST_TEST_EQ(sum.value(), 1); - BOOST_TEST_EQ(sum.large(), 1); - BOOST_TEST_EQ(sum.small(), 0); + BOOST_TEST_EQ(sum.large_part(), 1); + BOOST_TEST_EQ(sum.small_part(), 0); BOOST_TEST_EQ(str(sum), "sum(1 + 0)"s); BOOST_TEST_EQ(str(sum, 15, false), " sum(1 + 0)"s); BOOST_TEST_EQ(str(sum, 15, true), "sum(1 + 0) "s); +#include + + BOOST_TEST_EQ(sum.large(), 1); + BOOST_TEST_EQ(sum.small(), 0); + +#include + sum += 1e100; BOOST_TEST_EQ(sum, (s_t{1e100, 1})); ++sum; @@ -40,8 +47,8 @@ int main() { BOOST_TEST_EQ(sum, (s_t{0, 2})); BOOST_TEST_EQ(sum, 2); // correct answer BOOST_TEST_EQ(sum.value(), 2); - BOOST_TEST_EQ(sum.large(), 0); - BOOST_TEST_EQ(sum.small(), 2); + BOOST_TEST_EQ(sum.large_part(), 0); + BOOST_TEST_EQ(sum.small_part(), 2); sum = s_t{1e100, 1}; sum += s_t{1e100, 1}; diff --git a/test/histogram_fill_test.cpp b/test/histogram_fill_test.cpp index b4672c35..c49cab25 100644 --- a/test/histogram_fill_test.cpp +++ b/test/histogram_fill_test.cpp @@ -345,61 +345,6 @@ void run_tests(const std::vector& x, const std::vector& y, } } -void special_tests() { - // 1D growing (bug?) - { - using axis_type_1 = - axis::regular>; - using axis_type_2 = axis::regular; - using axis_type_3 = axis::integer; - using axis_type_4 = axis::category; - using axis_type_5 = axis::category; - - using axis_variant_type = - axis::variant; - - auto axes = std::vector({axis_type_1(10, 0, 1)}); - auto h = histogram>(axes); - auto h2 = h; - - std::vector f1({2}); - std::vector f2({-1}); - - h(2); - h(-1); - - h2.fill(f1); - h2.fill(f2); - - BOOST_TEST_EQ(h, h2); - BOOST_TEST_EQ(sum(h2), 2); - } - - // 1D growing (bug?) - { - using axis_type_1 = - axis::regular>; - - using axis_variant_type = axis::variant; - - auto axes = std::vector({axis_type_1(10, 0, 1)}); - auto h = histogram>(axes); - auto h2 = h; - - std::vector f1({2}); - std::vector f2({-1}); - - h(2); - h(-1); - - h2.fill(f1); - h2.fill(f2); - - BOOST_TEST_EQ(h, h2); - BOOST_TEST_EQ(sum(h2), 2); - } -} - int main() { std::mt19937 gen(1); std::normal_distribution<> id(0, 2); @@ -414,7 +359,5 @@ int main() { run_tests(x, y, w); run_tests(x, y, w); - special_tests(); - return boost::report_errors(); } diff --git a/test/issue_327_test.cpp b/test/issue_327_test.cpp index 1e4dac9d..b3fc7c6d 100644 --- a/test/issue_327_test.cpp +++ b/test/issue_327_test.cpp @@ -1,3 +1,9 @@ +// Copyright 2021 Hans Dembinski +// +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt +// or copy at http://www.boost.org/LICENSE_1_0.txt) + #include #include #include @@ -13,14 +19,40 @@ using arg_t = boost::variant2::variant, int>; int main() { using axis_type = bh::axis::regular; - using axis_variant = bh::axis::variant; + using axis_variant_type = bh::axis::variant; + + // 1D growing A + { + auto axes = std::vector({axis_type(10, 0, 1)}); + auto h = bh::make_histogram_with(bh::dense_storage(), axes); + auto h2 = h; + + h(-1); + + // used to crash, while growing B did not crash + std::vector vargs = {-1}; + h2.fill(vargs); + + BOOST_TEST_EQ(h, h2); + } + + // 1D growing B + { + auto axes = std::vector({axis_type(10, 0, 1)}); + auto h = bh::make_histogram_with(bh::dense_storage(), axes); + auto h2 = h; + + h(2); + h(-1); + + std::vector f1({2}); + std::vector f2({-1}); - auto axes = std::vector({axis_type(10, 0, 1)}); - auto h = bh::make_histogram_with(std::vector(), axes); - BOOST_TEST_EQ(h.rank(), 1); + h2.fill(f1); + h2.fill(f2); - std::vector vargs = {-1}; - h.fill(vargs); // CRASH, using h.fill(-1) or h.fill(args) does not crash. + BOOST_TEST_EQ(h, h2); + } return boost::report_errors(); } diff --git a/test/odr_test.cpp b/test/odr_test.cpp index 3fa08ead..8f2f4822 100644 --- a/test/odr_test.cpp +++ b/test/odr_test.cpp @@ -4,6 +4,12 @@ // (See accompanying file LICENSE_1_0.txt // or copy at http://www.boost.org/LICENSE_1_0.txt) +// The header windows.h and possibly others illegially do the following +#define small char +// which violates the C++ standard. We make sure here that including our headers work +// nevertheless by avoiding the preprocessing token `small`. For more details, see +// https://github.com/boostorg/histogram/issues/342 + // include all Boost.Histogram header here; see odr_main_test.cpp for details #include #include