Skip to content

Commit bb6491c

Browse files
committed
geometry: remove unwrap in BoundingBox::from_points
1 parent 3efef65 commit bb6491c

File tree

6 files changed

+65
-110
lines changed

6 files changed

+65
-110
lines changed

src/algorithms/hilbert_curve.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ fn hilbert_curve_partition(
2222
part_count: usize,
2323
order: usize,
2424
) {
25+
debug_assert!(!partition.is_empty()); // to compute the bounding box
26+
debug_assert_eq!(partition.len(), points.len());
27+
debug_assert_eq!(partition.len(), weights.len());
2528
debug_assert!(order < 64);
2629

2730
let compute_hilbert_index = hilbert_index_computer(points, order);
@@ -97,8 +100,9 @@ fn segment_to_segment(min: f64, max: f64, order: usize) -> impl Fn(f64) -> u64 {
97100
}
98101
}
99102

103+
/// Panics if `points` is empty.
100104
fn hilbert_index_computer(points: &[Point2D], order: usize) -> impl Fn(&Point2D) -> u64 {
101-
let mbr = OrientedBoundingBox::from_points(points);
105+
let mbr = OrientedBoundingBox::from_points(points).unwrap();
102106
let aabb = mbr.aabb();
103107
let x_mapping = segment_to_segment(aabb.p_min.x, aabb.p_max.x, order);
104108
let y_mapping = segment_to_segment(aabb.p_min.y, aabb.p_max.y, order);
@@ -326,6 +330,9 @@ where
326330
actual: self.order,
327331
});
328332
}
333+
if part_ids.is_empty() {
334+
return Ok(());
335+
}
329336
hilbert_curve_partition(
330337
part_ids,
331338
points.as_ref(),

src/algorithms/k_means.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -286,12 +286,16 @@ fn balanced_k_means_iter<const D: usize>(
286286
}
287287
}
288288

289-
// This is the main load balance routine. It handles:
290-
// - reordering the clusters according to their distance to a bounding box of all the points
291-
// - assigning each point to the closest cluster according to the effective distance
292-
// - checking partitions imbalance
293-
// - increasing of diminishing clusters influence based on their imbalance
294-
// - relaxing upper and lower bounds
289+
/// This is the main load balance routine. It handles:
290+
/// - reordering the clusters according to their distance to a bounding box of all the points
291+
/// - assigning each point to the closest cluster according to the effective distance
292+
/// - checking partitions imbalance
293+
/// - increasing of diminishing clusters influence based on their imbalance
294+
/// - relaxing upper and lower bounds
295+
///
296+
/// # Panics
297+
///
298+
/// Panics if `points` is empty.
295299
fn assign_and_balance<const D: usize>(
296300
points: &[PointND<D>],
297301
weights: &[f64],
@@ -316,11 +320,11 @@ fn assign_and_balance<const D: usize>(
316320
} = clusters;
317321
// compute the distances from each cluster center to the minimal
318322
// bounding rectangle of the set of points
319-
let mbr = OrientedBoundingBox::from_points(points);
323+
let obb = OrientedBoundingBox::from_points(points).unwrap();
320324
let distances_to_mbr = centers
321325
.par_iter()
322326
.zip(influences.par_iter())
323-
.map(|(center, influence)| mbr.distance_to_point(center) * influence)
327+
.map(|(center, influence)| obb.distance_to_point(center) * influence)
324328
.collect::<Vec<_>>();
325329

326330
let mut zipped = centers

src/algorithms/recursive_bisection.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -892,8 +892,11 @@ where
892892
W::Item: RcbWeight,
893893
W::Iter: rayon::iter::IndexedParallelIterator,
894894
{
895-
let mbr = OrientedBoundingBox::from_points(points);
896-
let points = points.par_iter().map(|p| mbr.obb_to_aabb(p));
895+
let obb = match OrientedBoundingBox::from_points(points) {
896+
Some(v) => v,
897+
None => return Ok(()),
898+
};
899+
let points = points.par_iter().map(|p| obb.obb_to_aabb(p));
897900
// When the rotation is done, we just apply RCB
898901
simple_rcb(partition, points, weights, n_iter, tolerance)
899902
}

src/algorithms/z_curve.rs

Lines changed: 8 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,25 @@ fn z_curve_partition<const D: usize>(
4848
DefaultAllocator: Allocator<f64, Const<D>, Const<D>, Buffer = ArrayStorage<f64, D, D>>
4949
+ Allocator<f64, DimDiff<Const<D>, Const<1>>>,
5050
{
51-
let max_order = (HASH_TYPE_MAX as f64).log(f64::from(2u32.pow(D as u32))) as u32;
51+
debug_assert_eq!(partition.len(), points.len());
52+
53+
let max_order = (HASH_TYPE_MAX as f64).log(f64::from(1 << D)) as u32;
5254
assert!(
5355
order <= max_order,
5456
"Cannot use the z-curve partition algorithm with an order > {} because it would currently overflow hashes capacity",
5557
max_order,
5658
);
5759

5860
// Bounding box used to construct Point hashes
59-
let mbr = OrientedBoundingBox::from_points(points);
61+
let obb = match OrientedBoundingBox::from_points(points) {
62+
Some(v) => v,
63+
None => return,
64+
};
6065

6166
let mut permutation: Vec<_> = (0..points.len()).into_par_iter().collect();
6267

6368
// reorder points
64-
z_curve_partition_recurse(points, order, &mbr, &mut permutation);
69+
z_curve_partition_recurse(points, order, &obb, &mut permutation);
6570

6671
let points_per_partition = points.len() / part_count;
6772
let remainder = points.len() % part_count;
@@ -133,64 +138,6 @@ fn z_curve_partition_recurse<const D: usize>(
133138
})
134139
}
135140

136-
// reorders a slice of Point3D in increasing z-curve order
137-
#[allow(unused)]
138-
pub(crate) fn z_curve_reorder<const D: usize>(points: &[PointND<D>], order: u32) -> Vec<usize>
139-
where
140-
Const<D>: DimSub<Const<1>>,
141-
DefaultAllocator: Allocator<f64, Const<D>, Const<D>, Buffer = ArrayStorage<f64, D, D>>
142-
+ Allocator<f64, DimDiff<Const<D>, Const<1>>>,
143-
{
144-
let max_order = (HASH_TYPE_MAX as f64).log(f64::from(2u32.pow(D as u32))) as u32;
145-
assert!(
146-
order <= max_order,
147-
"Cannot use the z-curve partition algorithm with an order > {} because it would currently overflow hashes capacity",
148-
max_order,
149-
);
150-
151-
let mut permu: Vec<_> = (0..points.len()).into_par_iter().collect();
152-
z_curve_reorder_permu(points, permu.as_mut_slice(), order);
153-
permu
154-
}
155-
156-
// reorders a slice of indices such that the associated array of Point3D is sorted
157-
// by increasing z-order
158-
#[allow(unused)]
159-
pub(crate) fn z_curve_reorder_permu<const D: usize>(
160-
points: &[PointND<D>],
161-
permu: &mut [usize],
162-
order: u32,
163-
) where
164-
Const<D>: DimSub<Const<1>>,
165-
DefaultAllocator: Allocator<f64, Const<D>, Const<D>, Buffer = ArrayStorage<f64, D, D>>
166-
+ Allocator<f64, DimDiff<Const<D>, Const<1>>>,
167-
{
168-
let mbr = OrientedBoundingBox::from_points(points);
169-
let hashes = permu
170-
.par_iter()
171-
.map(|idx| compute_hash(&points[*idx], order, &mbr))
172-
.collect::<Vec<_>>();
173-
174-
permu.par_sort_by_key(|idx| hashes[*idx]);
175-
}
176-
177-
fn compute_hash<const D: usize>(
178-
point: &PointND<D>,
179-
order: u32,
180-
mbr: &OrientedBoundingBox<D>,
181-
) -> HashType {
182-
let current_hash = mbr
183-
.region(point)
184-
.expect("Cannot compute the z-hash of a point outside of the current Mbr.");
185-
186-
if order == 0 {
187-
HashType::from(current_hash)
188-
} else {
189-
(2_u128.pow(D as u32)).pow(order) * HashType::from(current_hash)
190-
+ compute_hash(point, order - 1, &mbr.sub_mbr(current_hash))
191-
}
192-
}
193-
194141
/// # Z space-filling curve algorithm
195142
///
196143
/// The Z-curve uses space hashing to partition points. The points in the same part of a partition

src/geometry.rs

Lines changed: 28 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -28,22 +28,25 @@ impl<const D: usize> BoundingBox<D> {
2828
///
2929
/// This is the smallest rectangle (rectangular cuboid in 3D) that both
3030
/// contains all given points and is aligned with the axises.
31-
pub fn from_points<P>(points: P) -> Self
31+
///
32+
/// Returns `None` iff the given iterator is empty.
33+
pub fn from_points<P>(points: P) -> Option<Self>
3234
where
3335
P: IntoParallelIterator<Item = PointND<D>>,
3436
P::Iter: IndexedParallelIterator,
3537
{
3638
let points = points.into_par_iter();
37-
assert!(1 < points.len());
38-
39-
let (min, max) = points
39+
if points.len() == 0 {
40+
return None;
41+
}
42+
let (p_min, p_max) = points
4043
.fold_with(
4144
(
4245
PointND::<D>::from_element(std::f64::MAX),
4346
PointND::<D>::from_element(std::f64::MIN),
4447
),
4548
|(mut mins, mut maxs), vals| {
46-
for ((min, max), val) in mins.iter_mut().zip(maxs.iter_mut()).zip(vals.iter()) {
49+
for ((min, max), val) in mins.iter_mut().zip(maxs.iter_mut()).zip(&vals) {
4750
if *val < *min {
4851
*min = *val;
4952
}
@@ -59,23 +62,19 @@ impl<const D: usize> BoundingBox<D> {
5962
PointND::<D>::from_iterator(
6063
mins_left
6164
.into_iter()
62-
.zip(mins_right.into_iter())
65+
.zip(&mins_right)
6366
.map(|(left, right)| left.min(*right)),
6467
),
6568
PointND::<D>::from_iterator(
6669
maxs_left
6770
.into_iter()
68-
.zip(maxs_right.into_iter())
71+
.zip(&maxs_right)
6972
.map(|(left, right)| left.max(*right)),
7073
),
7174
)
7275
})
73-
.unwrap();
74-
75-
Self {
76-
p_min: min,
77-
p_max: max,
78-
}
76+
.unwrap(); // fold_with yields at least one element.
77+
Some(Self { p_min, p_max })
7978
}
8079

8180
fn center(&self) -> PointND<D> {
@@ -213,7 +212,7 @@ impl<const D: usize> OrientedBoundingBox<D> {
213212
/// The arbitrarily-oriented *minimum* bounding box.
214213
///
215214
/// The smallest box that contains all given points.
216-
pub fn from_points(points: &[PointND<D>]) -> Self
215+
pub fn from_points(points: &[PointND<D>]) -> Option<Self>
217216
where
218217
Const<D>: DimSub<Const<1>>,
219218
DefaultAllocator: Allocator<f64, Const<D>, Const<D>, Buffer = ArrayStorage<f64, D, D>>
@@ -225,13 +224,13 @@ impl<const D: usize> OrientedBoundingBox<D> {
225224
let base_change = householder_reflection(&vec);
226225
let obb_to_aabb = base_change.try_inverse().unwrap();
227226
let mapped = points.par_iter().map(|p| obb_to_aabb * p);
228-
let aabb = BoundingBox::from_points(mapped);
227+
let aabb = BoundingBox::from_points(mapped)?;
229228

230-
Self {
229+
Some(Self {
231230
aabb,
232231
aabb_to_obb: base_change,
233232
obb_to_aabb,
234-
}
233+
})
235234
}
236235

237236
/// Constructs a new Mbr that is a sub-Mbr of the current one.
@@ -365,23 +364,16 @@ mod tests {
365364
Point2D::from([4., 5.]),
366365
];
367366

368-
let aabb = BoundingBox::from_points(points);
367+
let aabb = BoundingBox::from_points(points).unwrap();
369368

370369
assert_ulps_eq!(aabb.p_min, Point2D::from([0., 0.]));
371370
assert_ulps_eq!(aabb.p_max, Point2D::from([5., 5.]));
372371
}
373372

374373
#[test]
375374
#[should_panic]
376-
fn test_aabb2d_invalid_input_1() {
377-
let _aabb = BoundingBox::<2>::from_points([]);
378-
}
379-
380-
#[test]
381-
#[should_panic]
382-
fn test_aabb2d_invalid_input_2() {
383-
let points = [Point2D::from([5., -9.2])];
384-
let _aabb = BoundingBox::from_points(points);
375+
fn test_aabb2d_invalid_input() {
376+
BoundingBox::<2>::from_points([]).unwrap();
385377
}
386378

387379
#[test]
@@ -422,29 +414,29 @@ mod tests {
422414

423415
#[test]
424416
fn test_obb_center() {
425-
let points = vec![
417+
let points = [
426418
Point2D::from([0., 1.]),
427419
Point2D::from([1., 0.]),
428420
Point2D::from([5., 6.]),
429421
Point2D::from([6., 5.]),
430422
];
431423

432-
let obb = OrientedBoundingBox::from_points(&points);
424+
let obb = OrientedBoundingBox::from_points(&points).unwrap();
433425

434426
let center = obb.center();
435427
assert_ulps_eq!(center, Point2D::from([3., 3.]))
436428
}
437429

438430
#[test]
439431
fn test_obb_contains() {
440-
let points = vec![
432+
let points = [
441433
Point2D::from([0., 1.]),
442434
Point2D::from([1., 0.]),
443435
Point2D::from([5., 6.]),
444436
Point2D::from([6., 5.]),
445437
];
446438

447-
let obb = OrientedBoundingBox::from_points(&points);
439+
let obb = OrientedBoundingBox::from_points(&points).unwrap();
448440

449441
assert!(!obb.contains(&Point2D::from([0., 0.])));
450442
assert!(obb.contains(&obb.center()));
@@ -458,14 +450,14 @@ mod tests {
458450

459451
#[test]
460452
fn test_obb_quadrant() {
461-
let points = vec![
453+
let points = [
462454
Point2D::from([0., 1.]),
463455
Point2D::from([1., 0.]),
464456
Point2D::from([5., 6.]),
465457
Point2D::from([6., 5.]),
466458
];
467459

468-
let obb = OrientedBoundingBox::from_points(&points);
460+
let obb = OrientedBoundingBox::from_points(&points).unwrap();
469461

470462
let none = obb.region(&Point2D::from([0., 0.]));
471463
let q1 = obb.region(&Point2D::from([1.2, 1.2]));
@@ -524,7 +516,7 @@ mod tests {
524516
Point3D::from([4., 5., 3.]),
525517
];
526518

527-
let aabb = BoundingBox::from_points(points);
519+
let aabb = BoundingBox::from_points(points).unwrap();
528520

529521
assert_ulps_eq!(aabb.p_min, Point3D::from([0., 0., -2.]));
530522
assert_ulps_eq!(aabb.p_max, Point3D::from([5., 5., 5.]));
@@ -551,7 +543,7 @@ mod tests {
551543

552544
#[test]
553545
fn test_obb_octant() {
554-
let points = vec![
546+
let points = [
555547
Point3D::from([0., 1., 0.]),
556548
Point3D::from([1., 0., 0.]),
557549
Point3D::from([5., 6., 0.]),
@@ -562,8 +554,7 @@ mod tests {
562554
Point3D::from([6., 5., 1.]),
563555
];
564556

565-
let obb = OrientedBoundingBox::from_points(&points);
566-
eprintln!("obb = {:#?}", obb);
557+
let obb = OrientedBoundingBox::from_points(&points).unwrap();
567558

568559
let none = obb.region(&Point3D::from([0., 0., 0.]));
569560
let octants = vec![

tools/src/bin/medit2svg.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,10 @@ where
229229
mesh.node_count(),
230230
)
231231
};
232-
let bb = coupe::BoundingBox::<2>::from_points(coordinates.par_iter().cloned());
232+
let bb = match coupe::BoundingBox::<2>::from_points(coordinates.par_iter().cloned()) {
233+
Some(v) => v,
234+
None => return Ok(()),
235+
};
233236
let xmin = bb.p_min[0];
234237
let xmax = bb.p_max[0];
235238
let ymin = bb.p_min[1];

0 commit comments

Comments
 (0)