diff --git a/build/CMakeLists.txt b/build/CMakeLists.txt index 20fa0516..1c54e92a 100644 --- a/build/CMakeLists.txt +++ b/build/CMakeLists.txt @@ -12,12 +12,12 @@ if(${CMAKE_BUILD_TYPE}) STRING(TOLOWER ${CMAKE_BUILD_TYPE} CMAKE_BUILD_TYPE) endif() -add_definitions(-Wall -pedantic -std=c++11) +add_definitions(-Wall -std=c++11) set(STD_LIBRARIES stdc++ m) if(BUILD_PYTHON) - find_package(Boost 1.55 REQUIRED + find_package(Boost 1.61 REQUIRED COMPONENTS iostreams serialization python) if(PYTHON_VERSION) find_package(PythonLibs ${PYTHON_VERSION} EXACT REQUIRED) @@ -57,8 +57,8 @@ endif() include_directories(../include ${Boost_INCLUDE_DIRS}) if(CMAKE_BUILD_TYPE STREQUAL "debug") - add_definitions(-O0 -g -fno-sanitize-recover=undefined,integer) - set(CMAKE_LD_FLAGS ${CMAKE_LD_FLAGS} -fno-sanitize-recover=undefined,integer) + add_definitions(-O0 -g -fno-sanitize-recover=undefined) + set(CMAKE_LD_FLAGS ${CMAKE_LD_FLAGS} -fno-sanitize-recover=undefined) message(STATUS "Build type: DEBUG [optimizations off]") elseif(CMAKE_BUILD_TYPE STREQUAL "cov") add_definitions(-O0 -g) diff --git a/include/boost/histogram/axis.hpp b/include/boost/histogram/axis.hpp index 4895885b..cd174456 100644 --- a/include/boost/histogram/axis.hpp +++ b/include/boost/histogram/axis.hpp @@ -7,7 +7,6 @@ #ifndef _BOOST_HISTOGRAM_AXIS_HPP_ #define _BOOST_HISTOGRAM_AXIS_HPP_ -#include #include #include #include @@ -31,12 +30,12 @@ public: /// Returns whether axis has extra overflow and underflow bins. inline bool uoflow() const { return shape_ > size_; } /// Returns the axis label, which is a name or description (not implemented for category_axis). - const char* label() const { return label_.c_str(); } + const std::string& label() const { return label_; } /// Change the label of an axis (not implemented for category_axis). - void label(const char* label) { label_ = detail::tiny_string(label); } + void label(const std::string& label) { label_ = label; } protected: - axis_with_label(unsigned n, const char* label, bool uoflow) : + axis_with_label(unsigned n, const std::string& label, bool uoflow) : size_(n), shape_(size_ + 2 * uoflow), label_(label) { if (n == 0) @@ -55,7 +54,7 @@ protected: private: int size_; int shape_; - detail::tiny_string label_; + std::string label_; template friend void serialize(Archive&, axis_with_label&, unsigned); @@ -95,7 +94,7 @@ public: * \param uoflow whether to add under-/overflow bins. */ regular_axis(unsigned n, double min, double max, - const char* label = nullptr, + const std::string& label = std::string(), bool uoflow = true) : axis_with_label(n, label, uoflow), min_(min), @@ -162,7 +161,7 @@ public: */ explicit polar_axis(unsigned n, double start = 0.0, - const char* label = nullptr) : + const std::string& label = std::string()) : axis_with_label(n, label, false), start_(start) {} @@ -215,7 +214,7 @@ public: */ explicit variable_axis(const std::initializer_list& x, - const char* label = nullptr, + const std::string& label = std::string(), bool uoflow = true) : axis_with_label(x.size() - 1, label, uoflow), x_(new double[x.size()]) @@ -227,7 +226,7 @@ public: } variable_axis(const std::vector& x, - const char* label = nullptr, + const std::string& label = std::string(), bool uoflow = true) : axis_with_label(x.size() - 1, label, uoflow), x_(new double[x.size()]) @@ -238,7 +237,7 @@ public: template variable_axis(Iterator begin, Iterator end, - const char* label = nullptr, + const std::string& label = std::string(), bool uoflow = true) : axis_with_label(std::distance(begin, end) - 1, label, uoflow), x_(new double[std::distance(begin, end)]) @@ -311,7 +310,7 @@ public: * \param max largest integer of the covered range. */ integer_axis(int min, int max, - const char* label = nullptr, + const std::string& label = std::string(), bool uoflow = true) : axis_with_label(max + 1 - min, label, uoflow), min_(min) @@ -357,12 +356,12 @@ private: */ class category_axis { public: - using value_type = const char*; + using value_type = const std::string&; template category_axis(Iterator begin, Iterator end) : size_(std::distance(begin, end)), - ptr_(new detail::tiny_string[size_]) + ptr_(new std::string[size_]) { if (size_ == 0) throw std::logic_error("at least one argument required"); @@ -374,7 +373,7 @@ public: * \param categories sequence of labeled categories. */ explicit - category_axis(const std::initializer_list& categories) : + category_axis(const std::initializer_list& categories) : category_axis(categories.begin(), categories.end()) {} @@ -390,8 +389,11 @@ public: {} category_axis(category_axis&&) = default; category_axis& operator=(const category_axis& other) { - category_axis tmp = other; - *this = std::move(tmp); + if (this != &other) { + size_ = other.size_; + ptr_.reset(new std::string[other.size_]); + std::copy(other.ptr_.get(), other.ptr_.get() + other.size_, ptr_.get()); + } return *this; } category_axis& operator=(category_axis&&) = default; @@ -408,19 +410,18 @@ public: return x; } /// Returns the category for the bin index. - const char* operator[](int idx) const - { return ptr_.get()[idx].c_str(); } + value_type operator[](int idx) const + { return ptr_.get()[idx]; } bool operator==(const category_axis& other) const { - if (size_ != other.size_) - return false; - return std::equal(ptr_.get(), ptr_.get() + size_, other.ptr_.get()); + return size_ == other.size_ && + std::equal(ptr_.get(), ptr_.get() + size_, other.ptr_.get()); } private: int size_; - std::unique_ptr ptr_; + std::unique_ptr ptr_; template friend void serialize(Archive&, category_axis&, unsigned); diff --git a/include/boost/histogram/detail/axis_visitor.hpp b/include/boost/histogram/detail/axis_visitor.hpp index a363edc6..5322f231 100644 --- a/include/boost/histogram/detail/axis_visitor.hpp +++ b/include/boost/histogram/detail/axis_visitor.hpp @@ -86,7 +86,7 @@ namespace detail { const int uoflow = a.uoflow(); // set stride to zero if 'j' is not in range, // this communicates the out-of-range condition to the caller - stride *= (j >= -uoflow) * (j < (a.bins() + uoflow)); + stride *= (j >= -uoflow) & (j < (a.bins() + uoflow)); j += (j < 0) * (a.bins() + 2); // wrap around if in < 0 out += j * stride; #pragma GCC diagnostic ignored "-Wstrict-overflow" @@ -103,6 +103,8 @@ namespace detail { template void operator()(const A& a) { + // the following is highly optimized code that runs in a hot loop; + // please measure the performance impact of changes // j is guaranteed to be in range [-1, bins] int j = a.index(x); j += (j < 0) * (a.bins() + 2); // wrap around if j < 0 diff --git a/include/boost/histogram/detail/tiny_string.hpp b/include/boost/histogram/detail/tiny_string.hpp deleted file mode 100644 index 4bb0d3ae..00000000 --- a/include/boost/histogram/detail/tiny_string.hpp +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2015-2016 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) - -#ifndef _BOOST_HISTOGRAM_DETAIL_TINY_STRING_HPP_ -#define _BOOST_HISTOGRAM_DETAIL_TINY_STRING_HPP_ - -#include -#include -#include -#include - -namespace boost { -namespace histogram { -namespace detail { - -class tiny_string : - boost::operators -{ -public: - tiny_string() = default; - - tiny_string(const tiny_string& other) : - tiny_string(other.c_str()) - {} - - tiny_string& operator=(const tiny_string& other) { - tiny_string tmp = other; - swap(tmp); - return *this; - } - - tiny_string(tiny_string&& other) noexcept = default; - tiny_string& operator=(tiny_string&& other) = default; - - tiny_string(const std::string& other) : - tiny_string(other.c_str()) - {} - - tiny_string& operator=(const std::string& other) { - tiny_string tmp(other.c_str()); - swap(tmp); - return *this; - } - - tiny_string(const char* other) - { - if (other) { - const auto n = std::strlen(other) + 1; - if (n > 1) { - ptr_.reset(new char[n]); - std::strcpy(ptr_.get(), other); - } - } - } - - tiny_string& operator=(const char* other) { - tiny_string tmp(other); - swap(tmp); - return *this; - } - - bool operator==(const tiny_string& other) const { - return std::strcmp(c_str(), other.c_str()) == 0; - } - - void swap(tiny_string& other) noexcept { - ptr_.swap(other.ptr_); - } - - std::size_t size() const { return ptr_ ? std::strlen(ptr_.get()) : 0; } - const char* c_str() const { return ptr_ ? ptr_.get() : ""; } -private: - std::unique_ptr ptr_; - - template - friend void serialize(Archiv&, tiny_string&, unsigned); -}; - -inline void swap(tiny_string& a, tiny_string& b) { a.swap(b); } - -inline std::ostream& operator<<(std::ostream& os, const tiny_string& s) -{ os << s.c_str(); return os; } - -} -} -} - -#endif diff --git a/include/boost/histogram/detail/utility.hpp b/include/boost/histogram/detail/utility.hpp index 47f29853..ebd8e188 100644 --- a/include/boost/histogram/detail/utility.hpp +++ b/include/boost/histogram/detail/utility.hpp @@ -8,6 +8,7 @@ #define _BOOST_HISTOGRAM_DETAIL_UTILITY_HPP_ #include +#include namespace boost { namespace histogram { @@ -30,9 +31,6 @@ namespace detail { escape(os, s.c_str()); } - inline - bool empty(const char* s) { return !s || s[0] == 0; } - inline bool empty(const std::string& s) { return s.empty(); } } diff --git a/include/boost/histogram/serialization.hpp b/include/boost/histogram/serialization.hpp index 1ef48683..f6b02c33 100644 --- a/include/boost/histogram/serialization.hpp +++ b/include/boost/histogram/serialization.hpp @@ -37,17 +37,6 @@ inline void serialize(Archive& ar, weight& wt, unsigned /* version */) ar & wt.w2; } -template -inline void serialize(Archive& ar, tiny_string& s, unsigned /* version */) -{ - auto n = s.size(); - ar & n; - if (Archive::is_loading::value) { - s.ptr_.reset(n ? new char[n+1] : nullptr); - } - ar & serialization::make_array(s.ptr_.get(), s.ptr_ ? n+1 : 0u); -} - } // NS detail template @@ -131,8 +120,7 @@ inline void serialize(Archive& ar, category_axis & axis, unsigned /* version */) { ar & axis.size_; if (Archive::is_loading::value) { - axis.ptr_.reset(axis.size_ ? - new detail::tiny_string[axis.size_] : nullptr); + axis.ptr_.reset(new std::string[axis.size_]); } ar & boost::serialization::make_array(axis.ptr_.get(), axis.size_); } diff --git a/src/python/axis.cpp b/src/python/axis.cpp index 77da6c88..0e8b82a9 100644 --- a/src/python/axis.cpp +++ b/src/python/axis.cpp @@ -37,14 +37,14 @@ variable_axis_init(python::tuple args, python::dict kwargs) { v.push_back(extract(args[i])); } - const char* label = nullptr; + std::string label; bool uoflow = true; while (len(kwargs) > 0) { tuple kv = kwargs.popitem(); std::string k = extract(kv[0]); object v = kv[1]; if (k == "label") - label = extract(v); + label = extract(v); else if (k == "uoflow") uoflow = extract(v); else { @@ -110,13 +110,13 @@ axis_len(const integer_axis& t) { } template -typename T::value_type +python::object axis_getitem(const T& t, int i) { if (i == axis_len(t)) { PyErr_SetString(PyExc_StopIteration, "no more"); python::throw_error_already_set(); } - return t[i]; + return python::object(t[i]); } template @@ -133,23 +133,24 @@ struct axis_suite : public python::def_visitor > { template static typename std::enable_if::value, void>::type - add_axis_label(Class& cl) { + label(Class& cl) { cl.add_property("label", - (const char*(U::*)() const) &U::label, - (void(U::*)(const char*)) &U::label, + make_function((const std::string&(U::*)() const) &U::label, + python::return_value_policy()), + (void(U::*)(const std::string&)) &U::label, "Name or description for the axis."); } template static typename std::enable_if::value, void>::type - add_axis_label(Class& cl) {} + label(Class& cl) {} template static void visit(Class& cl) { - add_axis_label(cl); + label(cl); cl.add_property("bins", &T::bins); cl.add_property("shape", &T::shape); cl.def("index", &T::index, @@ -191,9 +192,9 @@ void register_axis_types() "An axis for real-valued data and bins of equal width." "\nBinning is a O(1) operation.", no_init) - .def(init( + .def(init( (arg("self"), arg("bin"), arg("min"), arg("max"), - arg("label") = static_cast(nullptr), + arg("label") = std::string(), arg("uoflow") = true))) .def(axis_suite()) ; @@ -204,9 +205,9 @@ void register_axis_types() "\nsince the axis is circular and wraps around after 2pi." "\nBinning is a O(1) operation.", no_init) - .def(init( + .def(init( (arg("self"), arg("bin"), arg("start") = 0.0, - arg("label") = static_cast(nullptr)))) + arg("label") = std::string()))) .def(axis_suite()) ; @@ -216,10 +217,22 @@ void register_axis_types() "\nthe problem domain allows it, prefer a regular_axis.", no_init) .def("__init__", raw_function(variable_axis_init)) - .def(init, const char*, bool>()) + .def(init, const std::string&, bool>()) .def(axis_suite()) ; + class_("integer_axis", + "An axis for a contiguous range of integers." + "\nThere are no underflow/overflow bins for this axis." + "\nBinning is a O(1) operation.", + no_init) + .def(init( + (arg("self"), arg("min"), arg("max"), + arg("label") = std::string(), + arg("uoflow") = true))) + .def(axis_suite()) + ; + class_("category_axis", "An axis for enumerated categories. The axis stores the" "\ncategory labels, and expects that they are addressed" @@ -231,18 +244,6 @@ void register_axis_types() .def(init>()) .def(axis_suite()) ; - - class_("integer_axis", - "An axis for a contiguous range of integers." - "\nThere are no underflow/overflow bins for this axis." - "\nBinning is a O(1) operation.", - no_init) - .def(init( - (arg("self"), arg("min"), arg("max"), - arg("label") = static_cast(nullptr), - arg("uoflow") = true))) - .def(axis_suite()) - ; } } diff --git a/test/dynamic_histogram_test.cpp b/test/dynamic_histogram_test.cpp index d5386834..7916ed1c 100644 --- a/test/dynamic_histogram_test.cpp +++ b/test/dynamic_histogram_test.cpp @@ -294,7 +294,7 @@ int main() { // d2 { auto h = make_dynamic_histogram(regular_axis(2, -1, 1), - integer_axis(-1, 1, nullptr, false)); + integer_axis(-1, 1, "", false)); h.fill(-1, -1); h.fill(-1, 0); std::array ai = {{-1., -10.}}; @@ -345,7 +345,7 @@ int main() { // d2w { auto h = make_dynamic_histogram(regular_axis(2, -1, 1), - integer_axis(-1, 1, nullptr, false)); + integer_axis(-1, 1, "", false)); h.fill(-1, 0); // -> 0, 1 h.wfill(10, -1, -1); // -> 0, 0 h.wfill(5, -1, -10); // is ignored @@ -541,4 +541,4 @@ int main() { } return boost::report_errors(); -} \ No newline at end of file +}