From f5bbf6c7ddeb067d9616fb06b07d0ddbc8f524dd Mon Sep 17 00:00:00 2001 From: Ben FrantzDale Date: Wed, 3 Apr 2019 09:20:23 -0400 Subject: [PATCH 1/4] Fix IntervalTree's min() and max(). --- IntervalTree.h | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/IntervalTree.h b/IntervalTree.h index 5ed3405..5908f7f 100644 --- a/IntervalTree.h +++ b/IntervalTree.h @@ -160,15 +160,29 @@ class IntervalTree { Scalar min() const { assert(!empty()); - if (left) { return left->min(); } - return std::min_element(intervals.begin(), intervals.end(), - IntervalStartCmp())->start; + if (intervals.empty()) { + if (left) { return left->min(); } + return std::numeric_limits::infinity(); // Empty: Return inf as our min. + } + auto result = std::min_element(intervals.begin(), intervals.end(), + IntervalStartCmp())->start; + if (left) { + result = std::min(result, left->min()); + } + return result; } Scalar max() const { assert(!empty()); - if (right) { return right->max(); } - return std::max_element(intervals.begin(), intervals.end(), - IntervalStopCmp())->stop; + if (intervals.empty()) { + if (right) { return right->max(); } + return -std::numeric_limits::infinity(); // Empty: Return -inf as our max. + } + auto result = std::max_element(intervals.begin(), intervals.end(), + IntervalStopCmp())->stop; + if (right) { + result = std::max(result, right->max()); + } + return result; } // Call f on all intervals near the range [start, stop]: template From 43cbe04915d4c093161bf79dbbefe39f8451c5b3 Mon Sep 17 00:00:00 2001 From: Ben FrantzDale Date: Wed, 3 Apr 2019 09:45:15 -0400 Subject: [PATCH 2/4] Add tests for min/max. --- interval_tree_test.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/interval_tree_test.cpp b/interval_tree_test.cpp index fbfee2f..31f9753 100644 --- a/interval_tree_test.cpp +++ b/interval_tree_test.cpp @@ -186,3 +186,43 @@ int main(int argc, char**argv) { return Catch::Session().run( argc, argv ); } + +TEST_CASE( "Test min/max" ) { + typedef IntervalTree ITree; + { + ITree::interval_vector sanityIntervals; + sanityIntervals.push_back(ITree::interval(-100, 100, 0)); + sanityIntervals.push_back(ITree::interval(-90, -80, 1)); + ITree sanityTree(std::move(sanityIntervals), 16, 1, 1); + REQUIRE( sanityTree.min() == -100 ); + REQUIRE( sanityTree.max() == 100 ); + } + + { + ITree::interval_vector sanityIntervals; + sanityIntervals.push_back(ITree::interval(-100, 100, 0)); + sanityIntervals.push_back(ITree::interval(-90, -80, 1)); + sanityIntervals.push_back(ITree::interval(-10, 10, 2)); + ITree sanityTree(std::move(sanityIntervals), 16, 1, 1); + REQUIRE( sanityTree.min() == -100 ); + REQUIRE( sanityTree.max() == 100 ); + } + { + ITree::interval_vector sanityIntervals; + sanityIntervals.push_back(ITree::interval(-100, 100, 0)); + sanityIntervals.push_back(ITree::interval(-90, -80, 1)); + sanityIntervals.push_back(ITree::interval(-10, 10, 2)); + ITree sanityTree(std::move(sanityIntervals), 16, 1, 1); + REQUIRE( sanityTree.min() == -100 ); + REQUIRE( sanityTree.max() == 100 ); + } + { + ITree::interval_vector sanityIntervals; + sanityIntervals.push_back(ITree::interval(-100, 100, 0)); + sanityIntervals.push_back(ITree::interval(80, 90, 2)); + ITree sanityTree(std::move(sanityIntervals), 16, 1, 1); + REQUIRE( sanityTree.min() == -100 ); + REQUIRE( sanityTree.max() == 100 ); + } + +} From 7d9b5d2b76e3b87d0c0373b5f2f334961b75f51e Mon Sep 17 00:00:00 2001 From: Ben FrantzDale Date: Wed, 3 Apr 2019 09:27:50 -0400 Subject: [PATCH 3/4] Be smarter about min(). --- IntervalTree.h | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/IntervalTree.h b/IntervalTree.h index 5908f7f..ed22b16 100644 --- a/IntervalTree.h +++ b/IntervalTree.h @@ -160,28 +160,26 @@ class IntervalTree { Scalar min() const { assert(!empty()); + auto result = std::numeric_limits::max(); // If all else fails, return max as our min. + if (left) { result = left->min(); } if (intervals.empty()) { - if (left) { return left->min(); } - return std::numeric_limits::infinity(); // Empty: Return inf as our min. - } - auto result = std::min_element(intervals.begin(), intervals.end(), - IntervalStartCmp())->start; - if (left) { - result = std::min(result, left->min()); + return result; } + assert(std::is_sorted(intervals.begin(), intervals.end(), IntervalStartCmp())); + result = std::min(result, intervals.front().start); // These are sorted by start pos. return result; } Scalar max() const { assert(!empty()); + auto result = std::numeric_limits::lowest(); // If all else fails, return lowest as our max. + if (right) { result = right->max(); } if (intervals.empty()) { - if (right) { return right->max(); } - return -std::numeric_limits::infinity(); // Empty: Return -inf as our max. - } - auto result = std::max_element(intervals.begin(), intervals.end(), - IntervalStopCmp())->stop; - if (right) { - result = std::max(result, right->max()); + return result; } + // Our intervals are sorted by start pos, not end pos, so we have to check them all: + result = std::max(result, + std::max_element(intervals.begin(), intervals.end(), + IntervalStopCmp())->stop); return result; } // Call f on all intervals near the range [start, stop]: From c59e28645b43a3516f0809c6956a32c506321ea0 Mon Sep 17 00:00:00 2001 From: Ben FrantzDale Date: Wed, 3 Apr 2019 09:30:50 -0400 Subject: [PATCH 4/4] Scope min/max iterators, shrink_to_fit. --- IntervalTree.h | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/IntervalTree.h b/IntervalTree.h index ed22b16..e4857da 100644 --- a/IntervalTree.h +++ b/IntervalTree.h @@ -96,12 +96,15 @@ class IntervalTree { , right(nullptr) { --depth; - const auto minmaxStop = std::minmax_element(ivals.begin(), ivals.end(), - IntervalStopCmp()); - const auto minmaxStart = std::minmax_element(ivals.begin(), ivals.end(), - IntervalStartCmp()); - if (!ivals.empty()) { - center = (minmaxStart.first->start + minmaxStop.second->stop) / 2; + { // This scope is because afterward we sort and so these two iterators + // will stop meaing what they look say they mean, so we want them to go out of scope. + const auto minmaxStop = std::minmax_element(ivals.begin(), ivals.end(), + IntervalStopCmp()); + const auto minmaxStart = std::minmax_element(ivals.begin(), ivals.end(), + IntervalStartCmp()); + if (!ivals.empty()) { + center = (minmaxStart.first->start + minmaxStop.second->stop) / 2; + } } if (leftextent == 0 && rightextent == 0) { // sort intervals by start @@ -112,6 +115,7 @@ class IntervalTree { if (depth == 0 || (ivals.size() < minbucket && ivals.size() < maxbucket)) { std::sort(ivals.begin(), ivals.end(), IntervalStartCmp()); intervals = std::move(ivals); + intervals.shrink_to_fit(); assert(is_valid().first); return; } else {