Skip to content

Commit

Permalink
Feat better error handling (#36)
Browse files Browse the repository at this point in the history
Replaces most cases where this crate can panic with functions that return Result.
  • Loading branch information
davehadley authored Dec 27, 2023
1 parent 4cbf85b commit 762ec99
Show file tree
Hide file tree
Showing 43 changed files with 578 additions and 400 deletions.
1 change: 1 addition & 0 deletions ndhistogram/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# 0.10.0 (2023-12-03)

- Fix issue 33: Filling a histogram where the axis value is NaN not longer panics. NaN is mapped to the overflow bin where one exists or to no bin on axes without overflow bins.
- Most cases where the library could panic have been replaced with functions that return Result instead.
- Fix new clippy and compiler warnings.
- Minimum supported rust version is increased to 1.63.0.

Expand Down
1 change: 1 addition & 0 deletions ndhistogram/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ harness = false
[dependencies]
num-traits = "0.2.17"
rayon = {version="1.6.1", optional = true}
thiserror = "1.0.51"

[dependencies.serde]
version = "1.0.120"
Expand Down
24 changes: 12 additions & 12 deletions ndhistogram/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Please report any bugs in the [issues tracker](https://github.com/davehadley/ndh
use ndhistogram::{Histogram, ndhistogram, axis::Uniform};

// create a 1D histogram with 10 equally sized bins between -5 and 5
let mut hist = ndhistogram!(Uniform::new(10, -5.0, 5.0));
let mut hist = ndhistogram!(Uniform::new(10, -5.0, 5.0)?);
// fill this histogram with a single value
hist.fill(&1.0);
// fill this histogram with weights
Expand Down Expand Up @@ -140,14 +140,14 @@ User defined bin value types are possible by implementing the [Fill], [FillWith]
```rust
use ndhistogram::{Histogram, ndhistogram, axis::Uniform, value::Mean};
// Create a histogram whose bin values are i32
let mut hist = ndhistogram!(Uniform::new(10, -5.0, 5.0); i32);
let mut hist = ndhistogram!(Uniform::new(10, -5.0, 5.0)?; i32);
hist.fill_with(&1.0, 2);
let value: Option<&i32> = hist.value(&1.0);
assert_eq!(value, Some(&2));

// More complex value types beyond primitives are available
// "Mean" calculates the average of values it is filled with
let mut hist = ndhistogram!(Uniform::new(10, -5.0, 5.0); Mean);
let mut hist = ndhistogram!(Uniform::new(10, -5.0, 5.0)?; Mean);
hist.fill_with(&1.0, 1.0);
hist.fill_with(&1.0, 3.0);
assert_eq!(hist.value(&1.0).unwrap().mean(), 2.0);
Expand All @@ -163,7 +163,7 @@ assert_eq!(hist.value(&1.0).unwrap().mean(), 2.0);
```rust
use ndhistogram::{Histogram, ndhistogram, axis::Uniform};
// create a 2D histogram
let mut hist = ndhistogram!(Uniform::new(10, -5.0, 5.0), Uniform::new(10, -5.0, 5.0));
let mut hist = ndhistogram!(Uniform::new(10, -5.0, 5.0)?, Uniform::new(10, -5.0, 5.0)?);
// fill 2D histogram
hist.fill(&(1.0, 2.0));
// read back the histogram values
Expand Down Expand Up @@ -193,7 +193,7 @@ assert_eq!(hist.value(&"Red"), Some(&1.0));

```rust
use ndhistogram::{Histogram, ndhistogram, axis::Variable};
let mut hist = ndhistogram!(Variable::new(vec![0.0, 1.0, 3.0, 6.0]));
let mut hist = ndhistogram!(Variable::new(vec![0.0, 1.0, 3.0, 6.0])?);
for x in 0..6 {
hist.fill(&f64::from(x));
}
Expand All @@ -207,7 +207,7 @@ assert_eq!(hist.value(&3.0), Some(&3.0));
```rust
use std::f64::consts::PI;
use ndhistogram::{Histogram, ndhistogram, axis::UniformCyclic};
let mut hist = ndhistogram!(UniformCyclic::<f64>::new(10, 0.0, 2.0*PI));
let mut hist = ndhistogram!(UniformCyclic::<f64>::new(10, 0.0, 2.0*PI)?);
hist.fill(&PI);
hist.fill(&-PI);
// +pi and -pi are mapped onto the same value
Expand All @@ -221,9 +221,9 @@ assert_eq!(hist.value(&PI), Some(&2.0));
use ndhistogram::{Histogram, sparsehistogram, axis::Uniform};
// This histogram has 1e18 bins, too many to allocate with a normal histogram
let mut histogram_with_lots_of_bins = sparsehistogram!(
Uniform::new(1_000_000, -5.0, 5.0),
Uniform::new(1_000_000, -5.0, 5.0),
Uniform::new(1_000_000, -5.0, 5.0)
Uniform::new(1_000_000, -5.0, 5.0)?,
Uniform::new(1_000_000, -5.0, 5.0)?,
Uniform::new(1_000_000, -5.0, 5.0)?
);
histogram_with_lots_of_bins.fill(&(1.0, 2.0, 3.0));
// read back the filled value
Expand All @@ -236,8 +236,8 @@ assert!(histogram_with_lots_of_bins.value(&(0.0, 0.0, 0.0)).is_none());

```rust
use ndhistogram::{Histogram, ndhistogram, axis::Uniform};
let mut hist1 = ndhistogram!(Uniform::<f64>::new(10, -5.0, 5.0));
let mut hist2 = ndhistogram!(Uniform::<f64>::new(10, -5.0, 5.0));
let mut hist1 = ndhistogram!(Uniform::<f64>::new(10, -5.0, 5.0)?);
let mut hist2 = ndhistogram!(Uniform::<f64>::new(10, -5.0, 5.0)?);
hist1.fill_with(&0.0, 2.0);
hist2.fill_with(&0.0, 3.0);
let combined_hist = (hist1 + &hist2).expect("Axes are compatible");
Expand All @@ -248,7 +248,7 @@ let combined_hist = (hist1 + &hist2).expect("Axes are compatible");
```rust
use rayon::prelude::*;
use ndhistogram::{Histogram, ndhistogram, axis::Uniform};
let mut histogram = ndhistogram!(Uniform::<f64>::new(10, -5.0, 5.0));
let mut histogram = ndhistogram!(Uniform::<f64>::new(10, -5.0, 5.0)?);
let sum: f64 = histogram.par_iter().map(|bin| bin.value).sum();
// see also: par_iter_mut, par_values, par_values_mut.
assert_eq!(sum, 0.0);
Expand Down
23 changes: 14 additions & 9 deletions ndhistogram/benches/bench_fill.rs
Original file line number Diff line number Diff line change
@@ -1,41 +1,44 @@
use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion, Throughput};
use ndhistogram::axis::{Uniform, Variable};
use ndhistogram::value::WeightedMean;
use ndhistogram::{ndhistogram, sparsehistogram, Histogram};
use ndhistogram::{ndhistogram, sparsehistogram, Error, Histogram};
use rand::{prelude::StdRng, Rng, SeedableRng};

macro_rules! generate_fill_axis_benches {
($name:ident; $histoconstructor:ident; $numbins:ident; $axis:expr;) => {
paste::item! {

fn [< bench_ $name _single_fill_1d >] (c: &mut Criterion) {
fn [< bench_ $name _single_fill_1d >] (c: &mut Criterion) -> Result<(), Error> {
let $numbins = 10000;
let mut hist = $histoconstructor!($axis);
let mut rng = StdRng::seed_from_u64(12);
c.bench_function(stringify!([< bench_ $name _single_fill_1d >]), |b| {
b.iter(|| hist.fill(&black_box(rng.gen_range(-0.1..1.1))))
});
Ok(())
}

fn [< bench_ $name _single_fill_with_1d >] (c: &mut Criterion) {
fn [< bench_ $name _single_fill_with_1d >] (c: &mut Criterion) -> Result<(), Error> {
let $numbins = 10000;
let mut hist = $histoconstructor!($axis);
let mut rng = StdRng::seed_from_u64(12);
c.bench_function(stringify!([< bench_ $name _single_fill_with_1d >]), |b| {
b.iter(|| hist.fill_with(&black_box(rng.gen_range(-0.1..1.1)), black_box(rng.gen_range(0.0..2.0))))
});
Ok(())
}

fn [< bench_ $name _single_fill_with_weighted_1d >] (c: &mut Criterion) {
fn [< bench_ $name _single_fill_with_weighted_1d >] (c: &mut Criterion) -> Result<(), Error> {
let $numbins = 10000;
let mut hist = $histoconstructor!($axis; WeightedMean);
let mut rng = StdRng::seed_from_u64(12);
c.bench_function(stringify!([< bench_ $name _single_fill_with_weighted_1d >]), |b| {
b.iter(|| hist.fill_with_weighted(&black_box(rng.gen_range(-0.1..1.1)), black_box(rng.gen_range(0.0..2.0)), black_box(rng.gen_range(0.0..2.0))))
});
Ok(())
}

fn [< bench_ $name _iter_fill_2d_vs_num_fills >](c: &mut Criterion) {
fn [< bench_ $name _iter_fill_2d_vs_num_fills >](c: &mut Criterion) -> Result<(), Error> {
let $numbins = 1000;
let mut hist = $histoconstructor!($axis, $axis);
let mut rng = StdRng::seed_from_u64(12);
Expand All @@ -49,9 +52,10 @@ macro_rules! generate_fill_axis_benches {
b.iter(|| data.iter().for_each(|it| hist.fill(it)))
});
}
Ok(())
}

fn [< bench_ $name _iter_fill_2d_vs_num_bins >](c: &mut Criterion) {
fn [< bench_ $name _iter_fill_2d_vs_num_bins >](c: &mut Criterion) -> Result<(), Error> {
let mut group = c.benchmark_group(stringify!([< bench_ $name _iter_fill_2d_vs_num_bins >]));
for size in [10, 100, 1000, 10000] {
let $numbins = size;
Expand All @@ -65,6 +69,7 @@ macro_rules! generate_fill_axis_benches {
b.iter(|| data.iter().for_each(|it| hist.fill(it)))
});
}
Ok(())
}

criterion_group!(
Expand All @@ -80,10 +85,10 @@ macro_rules! generate_fill_axis_benches {
};
}

generate_fill_axis_benches! {vec_uniform; ndhistogram; numbins; Uniform::new(numbins, 0.0, 1.0);}
generate_fill_axis_benches! {vec_uniform; ndhistogram; numbins; Uniform::new(numbins, 0.0, 1.0)?;}

generate_fill_axis_benches! {sparse_uniform; sparsehistogram; numbins; Uniform::new(numbins, 0.0, 1.0);}
generate_fill_axis_benches! {sparse_uniform; sparsehistogram; numbins; Uniform::new(numbins, 0.0, 1.0)?;}

generate_fill_axis_benches! {vec_variable; ndhistogram; numbins; Variable::new((0..numbins+1).map(|it| (it as f64)/(numbins as f64)).collect::<Vec<f64>>());}
generate_fill_axis_benches! {vec_variable; ndhistogram; numbins; Variable::new((0..numbins+1).map(|it| (it as f64)/(numbins as f64)).collect::<Vec<f64>>())?;}

criterion_main!(bench_vec_uniform, bench_sparse_uniform, bench_vec_variable);
7 changes: 4 additions & 3 deletions ndhistogram/examples/simple_histogram_1d.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
extern crate ndhistogram;
use ndhistogram::axis::Uniform;
use ndhistogram::{ndhistogram, Histogram};
use ndhistogram::{ndhistogram, Error, Histogram};

fn main() {
let mut hist = ndhistogram!(Uniform::new(10, 0.0, 1.0));
fn main() -> Result<(), Error> {
let mut hist = ndhistogram!(Uniform::new(10, 0.0, 1.0)?);

// fill a single number
hist.fill(&0.0);
Ok(())
}
7 changes: 4 additions & 3 deletions ndhistogram/examples/simple_histogram_2d.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
extern crate ndhistogram;
use ndhistogram::axis::Uniform;
use ndhistogram::{ndhistogram, Histogram};
use ndhistogram::{ndhistogram, Error, Histogram};

fn main() {
let mut hist = ndhistogram!(Uniform::new(10, 0.0, 1.0), Uniform::new(10, 1.0, 2.0));
fn main() -> Result<(), Error> {
let mut hist = ndhistogram!(Uniform::new(10, 0.0, 1.0)?, Uniform::new(10, 1.0, 2.0)?);

// fill a single number
hist.fill(&(0.0, 1.0));
Ok(())
}
37 changes: 19 additions & 18 deletions ndhistogram/src/axis/uniform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use std::fmt::{Debug, Display};

use num_traits::{Float, Num, NumCast, NumOps};

use crate::error::AxisError;

use super::{Axis, BinInterval};

/// An axis with equal sized bins.
Expand All @@ -18,12 +20,13 @@ use super::{Axis, BinInterval};
/// ```rust
/// use ndhistogram::{ndhistogram, Histogram};
/// use ndhistogram::axis::{Axis, Uniform, BinInterval};
/// let hist = ndhistogram!(Uniform::new(10, -5.0, 5.0));
/// # fn main() -> Result<(), ndhistogram::Error> {
/// let hist = ndhistogram!(Uniform::new(10, -5.0, 5.0)?);
/// let axis = &hist.axes().as_tuple().0;
/// assert_eq!(axis.bin(0), Some(BinInterval::underflow(-5.0)));
/// assert_eq!(axis.bin(1), Some(BinInterval::new(-5.0, -4.0)));
/// assert_eq!(axis.bin(11), Some(BinInterval::overflow(5.0)));
///
/// # Ok(()) }
/// ```
#[derive(Default, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand All @@ -42,47 +45,45 @@ where
///
/// Only implemented for [Float]. Use [Uniform::with_step_size] for integers.
///
/// # Panics
/// Panics if num bins == 0 or low == high.
pub fn new(num: usize, low: T, high: T) -> Self
pub fn new(num: usize, low: T, high: T) -> Result<Self, AxisError>
where
T: Float,
{
if num == 0 {
panic!("Invalid axis num bins ({})", num);
return Err(AxisError::InvalidNumberOfBins);
}
if low == high {
panic!("Invalid axis range (low == high)");
return Err(AxisError::InvalidAxisRange);
}
let (low, high) = if low > high { (high, low) } else { (low, high) };
let step = (high - low) / T::from(num).expect("");
Self {
let step = (high - low) / T::from(num).ok_or(AxisError::InvalidNumberOfBins)?;
Ok(Self {
num,
low,
high,
step,
}
})
}

/// Factory method to create an axis with num uniformly spaced bins in the range [low, low+num*step). Under/overflow bins cover values outside this range.
///
/// # Panics
/// Panics if num bins == 0 or step <= 0.
pub fn with_step_size(num: usize, low: T, step: T) -> Self {
let high = T::from(num).expect("num bins can be converted to coordinate type") * step + low;
/// The number of bins and step size must both be greater than zero, otherwise an error is returned.
/// The number of bins must be representable in the type T, otherwise an error is returned.
pub fn with_step_size(num: usize, low: T, step: T) -> Result<Self, AxisError> {
let high = T::from(num).ok_or(AxisError::InvalidNumberOfBins)? * step + low;
if num == 0 {
panic!("Invalid axis num bins ({})", num);
return Err(AxisError::InvalidNumberOfBins);
}
if step <= T::zero() {
panic!("Invalid step size. Step size must be greater than zero.");
return Err(AxisError::InvalidStepSize);
}
let (low, high) = if low > high { (high, low) } else { (low, high) };
Self {
Ok(Self {
num,
low,
high,
step,
}
})
}
}

Expand Down
33 changes: 19 additions & 14 deletions ndhistogram/src/axis/uniformcyclic.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use crate::error::AxisError;

use super::{Axis, BinInterval, UniformNoFlow};
use std::fmt::{Debug, Display};

Expand All @@ -14,7 +16,8 @@ use num_traits::{Float, Num, NumCast, NumOps};
/// ```
/// use ndhistogram::{ndhistogram, Histogram};
/// use ndhistogram::axis::{Axis, BinInterval, UniformCyclic};
/// let mut hist = ndhistogram!(UniformCyclic::new(4, 0.0, 360.0));
/// # fn main() -> Result<(), ndhistogram::Error> {
/// let mut hist = ndhistogram!(UniformCyclic::new(4, 0.0, 360.0)?);
/// hist.fill(& 45.0 ); // Add entry at 45 degrees
/// hist.fill(&(45.0 + 360.0)); // Add entry at 45 degrees + one whole turn
/// hist.fill(&(45.0 - 360.0)); // Add entry at 45 degrees + one whole turn backwards
Expand All @@ -23,20 +26,24 @@ use num_traits::{Float, Num, NumCast, NumOps};
/// // Lookup also wraps around
/// assert_eq!(hist.value(&(45.0 + 360.0)), Some(&3.0));
/// assert_eq!(hist.value(&(45.0 - 360.0)), Some(&3.0));
/// # Ok(()) }
/// ```
/// Time of day
/// ```
/// use ndhistogram::{ndhistogram, Histogram};
/// use ndhistogram::axis::{Axis, BinInterval, UniformCyclic};
///
/// # fn main() -> Result<(), ndhistogram::Error> {
/// let bins_per_day = 24;
/// let hours_per_bin = 1;
/// let start_at_zero = 0;
/// let four_pm = 16;
/// let mut hist = ndhistogram!(UniformCyclic::with_step_size(
/// bins_per_day, start_at_zero, hours_per_bin
/// ));
/// )?);
/// hist.fill(&40); // The 40th hour of the week ...
/// assert_eq!(hist.value(&four_pm), Some(&1.0)); // ... is at 4 pm.
/// # Ok(()) }
/// ````
#[derive(Default, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand All @@ -55,24 +62,22 @@ where
///
/// For floating point types, infinities and NaN do not map to any bin.
///
/// # Panics
/// Panics under the same conditions as [UniformNoFlow::new].
pub fn new(nbins: usize, low: T, high: T) -> Self
/// The parameters have the same constraints as [UniformNoFlow::new], otherwise an error in returned.
pub fn new(nbins: usize, low: T, high: T) -> Result<Self, AxisError>
where
T: Float,
{
Self {
axis: UniformNoFlow::new(nbins, low, high),
}
Ok(Self {
axis: UniformNoFlow::new(nbins, low, high)?,
})
}

/// Create a wrap-around axis with `nbins` uniformly-spaced bins in the range `[low, low+num*step)`.
/// # Panics
/// Panics under the same conditions as [UniformNoFlow::new].
pub fn with_step_size(nbins: usize, low: T, step: T) -> Self {
Self {
axis: UniformNoFlow::with_step_size(nbins, low, step),
}
/// The parameters have the same constraints as [UniformNoFlow::new], otherwise an error is returned.
pub fn with_step_size(nbins: usize, low: T, step: T) -> Result<Self, AxisError> {
Ok(Self {
axis: UniformNoFlow::with_step_size(nbins, low, step)?,
})
}
}

Expand Down
Loading

0 comments on commit 762ec99

Please sign in to comment.