Skip to content

Commit fe0f2f8

Browse files
committed
fix(rtree): clamp content differences to prevent FMA-induced numerical instability
Might fix #1452
1 parent 8b6e213 commit fe0f2f8

5 files changed

Lines changed: 36 additions & 17 deletions

File tree

include/boost/geometry/index/detail/algorithms/content.hpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,19 @@ typename default_content_result<Indexable>::type content(Indexable const& b)
112112
>::apply(b);
113113
}
114114

115+
// Returns the content increase when expanding 'original' to 'expanded'.
116+
// Precondition: 'original' is covered by 'expanded'. Under this precondition
117+
// the result is mathematically non-negative, so clamping to 0 is valid and
118+
// guards against floating-point contraction artifacts (e.g. GCC FMA fusion
119+
// across function boundaries) that can make the difference negative or
120+
// spuriously non-zero for equal boxes. See GitHub issue #1452.
121+
template <typename Box>
122+
typename default_content_result<Box>::type content_diff(Box const& expanded, Box const& original)
123+
{
124+
using content_type = typename default_content_result<Box>::type;
125+
return (std::max)(content_type(0), content(expanded) - content(original));
126+
}
127+
115128
#if defined(BOOST_GCC)
116129
#pragma GCC pop_options
117130
#endif

include/boost/geometry/index/detail/rtree/linear/redistribute_elements.hpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#ifndef BOOST_GEOMETRY_INDEX_DETAIL_RTREE_LINEAR_REDISTRIBUTE_ELEMENTS_HPP
1717
#define BOOST_GEOMETRY_INDEX_DETAIL_RTREE_LINEAR_REDISTRIBUTE_ELEMENTS_HPP
1818

19+
#include <algorithm>
1920
#include <type_traits>
2021

2122
#include <boost/core/ignore_unused.hpp>
@@ -432,8 +433,10 @@ struct redistribute_elements<MembersHolder, linear_tag>
432433
content_type enlarged_content1 = index::detail::content(enlarged_box1);
433434
content_type enlarged_content2 = index::detail::content(enlarged_box2);
434435

435-
content_type content_increase1 = enlarged_content1 - content1;
436-
content_type content_increase2 = enlarged_content2 - content2;
436+
content_type const content_increase1
437+
= (std::max)(content_type(0), enlarged_content1 - content1);
438+
content_type const content_increase2
439+
= (std::max)(content_type(0), enlarged_content2 - content2);
437440

438441
// choose group which box content have to be enlarged least or has smaller content or has fewer elements
439442
if ( content_increase1 < content_increase2 ||

include/boost/geometry/index/detail/rtree/quadratic/redistribute_elements.hpp

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,10 @@ inline void pick_seeds(Elements const& elements,
7474

7575
bounded_indexable_view bounded_ind1(ind1, strategy);
7676
bounded_indexable_view bounded_ind2(ind2, strategy);
77-
content_type free_content = ( index::detail::content(enlarged_box)
78-
- index::detail::content(bounded_ind1) )
79-
- index::detail::content(bounded_ind2);
77+
content_type free_content = (std::max)(content_type(0),
78+
( index::detail::content(enlarged_box)
79+
- index::detail::content(bounded_ind1) )
80+
- index::detail::content(bounded_ind2));
8081

8182
if ( greatest_free_content < free_content )
8283
{
@@ -273,7 +274,7 @@ struct redistribute_elements<MembersHolder, quadratic_tag>
273274
typedef typename boost::iterator_value<It>::type element_type;
274275
typedef typename rtree::element_indexable_type<element_type, translator_type>::type indexable_type;
275276

276-
content_type greatest_content_incrase_diff = 0;
277+
content_type greatest_content_increase_diff = 0;
277278
It out_it = first;
278279
out_content_increase1 = 0;
279280
out_content_increase2 = 0;
@@ -291,18 +292,20 @@ struct redistribute_elements<MembersHolder, quadratic_tag>
291292
content_type enlarged_content1 = index::detail::content(enlarged_box1);
292293
content_type enlarged_content2 = index::detail::content(enlarged_box2);
293294

294-
content_type content_incrase1 = (enlarged_content1 - content1);
295-
content_type content_incrase2 = (enlarged_content2 - content2);
295+
content_type const content_increase1
296+
= (std::max)(content_type(0), enlarged_content1 - content1);
297+
content_type const content_increase2
298+
= (std::max)(content_type(0), enlarged_content2 - content2);
296299

297-
content_type content_incrase_diff = content_incrase1 < content_incrase2 ?
298-
content_incrase2 - content_incrase1 : content_incrase1 - content_incrase2;
300+
content_type content_increase_diff = content_increase1 < content_increase2 ?
301+
content_increase2 - content_increase1 : content_increase1 - content_increase2;
299302

300-
if ( greatest_content_incrase_diff < content_incrase_diff )
303+
if ( greatest_content_increase_diff < content_increase_diff )
301304
{
302-
greatest_content_incrase_diff = content_incrase_diff;
305+
greatest_content_increase_diff = content_increase_diff;
303306
out_it = el_it;
304-
out_content_increase1 = content_incrase1;
305-
out_content_increase2 = content_incrase2;
307+
out_content_increase1 = content_increase1;
308+
out_content_increase2 = content_increase2;
306309
}
307310
}
308311

include/boost/geometry/index/detail/rtree/rstar/choose_next_node.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ class choose_next_node<MembersHolder, choose_by_overlap_diff_tag>
117117

118118
// areas difference
119119
content_type content = index::detail::content(box_exp);
120-
content_type content_diff = content - index::detail::content(ch_i.first);
120+
content_type content_diff = index::detail::content_diff(box_exp, ch_i.first);
121121

122122
children_contents[i].set(i, content, content_diff);
123123

@@ -246,7 +246,7 @@ class choose_next_node<MembersHolder, choose_by_overlap_diff_tag>
246246

247247
// areas difference
248248
content_type content = index::detail::content(box_exp);
249-
content_type content_diff = content - index::detail::content(ch_i.first);
249+
content_type content_diff = index::detail::content_diff(box_exp, ch_i.first);
250250

251251
// update the result
252252
if ( content_diff < smallest_content_diff ||

include/boost/geometry/index/detail/rtree/visitors/insert.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class choose_next_node<MembersHolder, choose_by_content_diff_tag>
8989

9090
// areas difference
9191
content_type content = index::detail::content(box_exp);
92-
content_type content_diff = content - index::detail::content(ch_i.first);
92+
content_type content_diff = index::detail::content_diff(box_exp, ch_i.first);
9393

9494
// update the result
9595
if ( content_diff < smallest_content_diff ||

0 commit comments

Comments
 (0)