Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat better error handling #36

Merged
merged 14 commits into from
Dec 27, 2023
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 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 @@
/// ```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 @@
///
/// 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);

Check warning on line 53 in ndhistogram/src/axis/uniform.rs

View check run for this annotation

Codecov / codecov/patch

ndhistogram/src/axis/uniform.rs#L53

Added line #L53 was not covered by tests
}
if low == high {
panic!("Invalid axis range (low == high)");
return Err(AxisError::InvalidAxisRange);

Check warning on line 56 in ndhistogram/src/axis/uniform.rs

View check run for this annotation

Codecov / codecov/patch

ndhistogram/src/axis/uniform.rs#L56

Added line #L56 was not covered by tests
}
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);

Check warning on line 75 in ndhistogram/src/axis/uniform.rs

View check run for this annotation

Codecov / codecov/patch

ndhistogram/src/axis/uniform.rs#L75

Added line #L75 was not covered by tests
}
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
Loading