diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cfc3dfd..2e4e3ca3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,13 @@ All significant changes to this project will be documented in this file. * `BloomFilter::invert` is removed; use `BloomFilter::difference` for approximate A-not-B. The result excludes items in the right filter, but hash collisions can also remove items unique to the left filter. * `FrequentItemsSketch::is_empty` now returns `false` when the stream weight is nonzero, even if no items are retained. Use `num_active_items() == 0` to test for zero retained items. * `ReqSketch` queries now use `datasketches::common::SearchCriteria` instead of `datasketches::req::SearchCriteria`. `SearchCriteria` no longer implements `Default`; explicitly choose `Inclusive` or `Exclusive`. +* `ReqSketch` and `req::SortedView` queries now distinguish empty sketches from invalid arguments. `rank` returns `Option`; `quantile`, `quantiles`, `cdf`, and `pmf`, where available, return `Result, Error>`. Handle `None` for valid queries on an empty sketch, and propagate or handle `InvalidArgument` for invalid ranks or split points, which are checked before emptiness. +* `TDigest` and `TDigestMut` rank, quantile, CDF, and PMF queries now return `Result, Error>`. Invalid arguments return `InvalidArgument` instead of panicking, including on empty digests. Propagate or handle the `Result` before handling `None` for an empty digest; `min_value` and `max_value` still return `Option`. ### New features * Add `HllSketch::composite_estimate` and `HllUnion::composite_estimate` to query cardinality without relying on HIP update history, matching Java's `getCompositeEstimate()`. -* `KllSketch` is now available behind the `kll` feature, with rank, quantile, PMF, and CDF queries, merging, serialization, custom ordered item types, and a `KllFloat` adapter for non-NaN floating-point values. +* `KllSketch` is now available behind the `kll` feature, with rank, quantile, PMF, and CDF queries, merging, serialization, custom ordered item types, and a `KllFloat` adapter for non-NaN floating-point values. Queries distinguish an empty sketch (`None`) from invalid arguments (`InvalidArgument`). * Add `TDigestMut::quantiles` and `TDigest::quantiles` for querying several ranks in one centroid scan while preserving input order. * Add `FromIterator` as an alternative to incremental `TDigestMut::merge` calls for combining owned partial sketches in a single compression pass. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 841f8509..8d9f5bad 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -80,6 +80,13 @@ cargo bench --package benchmarks --bench benchmarks -- cpc::serde - Describe types with noun phrases and API behavior with third-person present-tense verbs such as `Creates`, `Updates`, and `Returns`. - End summary sentences with punctuation, and format Rust identifiers, literals, and numeric ranges as inline code. - Put contract sections and compatibility notes before examples. When applicable, order sections as `# Errors`, `# Panics`, and `# Examples`. Include only sections that describe an actual contract. +- Document return values and error conditions on the relevant methods rather than repeating them in crate or module overviews. +- Keep imports visible in doctests. A trailing hidden `Ok(...)` may be used to support `?`. + +## Query semantics + +- Validate query arguments before handling an empty sketch. Invalid arguments return `InvalidArgument` regardless of whether the sketch is empty. +- Return a meaningful result when one exists, such as zero, `false`, or an empty collection. Otherwise, use `Option` for an absent result, combined with `Result` when argument validation can fail. ## Visibility diff --git a/benchmarks/tdigest/aggregation.rs b/benchmarks/tdigest/aggregation.rs index da17069d..dd418235 100644 --- a/benchmarks/tdigest/aggregation.rs +++ b/benchmarks/tdigest/aggregation.rs @@ -127,4 +127,5 @@ fn final_quantile(mut merged: TDigestMut) -> Option { TDigestMut::deserialize(black_box(&bytes)) .unwrap() .quantile(black_box(0.9)) + .unwrap() } diff --git a/benchmarks/tdigest/compress.rs b/benchmarks/tdigest/compress.rs index 3fd4cb63..ead19006 100644 --- a/benchmarks/tdigest/compress.rs +++ b/benchmarks/tdigest/compress.rs @@ -31,14 +31,14 @@ fn initial_buffer(bencher: Bencher) { bencher .counter(ItemsCount::new(values.len())) .with_inputs(|| digest.clone()) - .bench_local_values(|mut digest| black_box(digest.rank(0.5))); + .bench_local_values(|mut digest| black_box(digest.rank(0.5).unwrap())); } #[divan::bench] fn unmerged_tail(bencher: Bencher) { let values = values(3_280); let mut digest = build_mut_digest(&values[..1_640]); - black_box(digest.rank(0.5)); + black_box(digest.rank(0.5).unwrap()); for &value in &values[1_640..] { digest.update(value); } @@ -46,7 +46,7 @@ fn unmerged_tail(bencher: Bencher) { bencher .counter(ItemsCount::new(1_640_usize)) .with_inputs(|| digest.clone()) - .bench_local_values(|mut digest| black_box(digest.rank(0.5))); + .bench_local_values(|mut digest| black_box(digest.rank(0.5).unwrap())); } #[divan::bench] diff --git a/benchmarks/tdigest/merge.rs b/benchmarks/tdigest/merge.rs index 12f6f8b0..d8ae2739 100644 --- a/benchmarks/tdigest/merge.rs +++ b/benchmarks/tdigest/merge.rs @@ -34,8 +34,8 @@ fn merge(bencher: Bencher) { let values = values(200_000); let mut left = build_mut_digest(&values[..100_000]); let mut right = build_mut_digest(&values[100_000..]); - black_box(left.rank(0.5)); - black_box(right.rank(0.5)); + black_box(left.rank(0.5).unwrap()); + black_box(right.rank(0.5).unwrap()); bencher .counter(ItemsCount::new(values.len())) @@ -65,7 +65,7 @@ fn small_partials(bencher: Bencher) { let partials = partial_digests(64, SMALL_ROWS_PER_PARTIAL) .into_iter() .map(|mut digest| { - black_box(digest.quantile(0.5)); + black_box(digest.quantile(0.5).unwrap()); digest }) .collect::>(); @@ -86,7 +86,7 @@ fn partials(bencher: Bencher) { let partials = partial_digests_with(DEFAULT_DIGEST_K, 64, ROWS_PER_PARTIAL) .into_iter() .map(|mut digest| { - black_box(digest.quantile(0.5)); + black_box(digest.quantile(0.5).unwrap()); digest }) .collect::>(); @@ -107,7 +107,7 @@ fn partials_from_iter(bencher: Bencher) { let partials = partial_digests_with(DEFAULT_DIGEST_K, 64, ROWS_PER_PARTIAL) .into_iter() .map(|mut digest| { - black_box(digest.quantile(0.5)); + black_box(digest.quantile(0.5).unwrap()); digest }) .collect::>(); @@ -125,7 +125,7 @@ fn partials_from_iter_cloned(bencher: Bencher) { let partials = partial_digests_with(DEFAULT_DIGEST_K, 64, ROWS_PER_PARTIAL) .into_iter() .map(|mut digest| { - black_box(digest.quantile(0.5)); + black_box(digest.quantile(0.5).unwrap()); digest }) .collect::>(); @@ -141,7 +141,7 @@ fn partials_from_iter_finalize(bencher: Bencher, additional_rows: usize) { let partials = partial_digests_with(DEFAULT_DIGEST_K, 64, ROWS_PER_PARTIAL) .into_iter() .map(|mut digest| { - black_box(digest.quantile(0.5)); + black_box(digest.quantile(0.5).unwrap()); digest }) .collect::>(); @@ -188,7 +188,7 @@ fn serialized_partials(bencher: Bencher, rows_per_partial: usize) { let partial = TDigestMut::deserialize(partial).unwrap(); merged.merge(&partial); } - black_box(merged.quantile(0.5)) + black_box(merged.quantile(0.5).unwrap()) }); } @@ -208,7 +208,7 @@ fn serialized_overlapping_partials(bencher: Bencher) { let partial = TDigestMut::deserialize(partial).unwrap(); merged.merge(&partial); } - black_box(merged.quantile(0.5)) + black_box(merged.quantile(0.5).unwrap()) }); } @@ -224,7 +224,7 @@ fn serialized_partials_from_iter(bencher: Bencher, overlapping: bool) { .map(|partial| TDigestMut::deserialize(partial)) .collect::>() .unwrap(); - black_box(merged.quantile(0.5)) + black_box(merged.quantile(0.5).unwrap()) }); } @@ -244,7 +244,7 @@ fn serialized_partials_in_batches(bencher: Bencher, overlapping: bool) { .unwrap(); merged.merge(&batch); } - black_box(merged.quantile(0.5)) + black_box(merged.quantile(0.5).unwrap()) }); } diff --git a/benchmarks/tdigest/query.rs b/benchmarks/tdigest/query.rs index 338179e8..76b05ac8 100644 --- a/benchmarks/tdigest/query.rs +++ b/benchmarks/tdigest/query.rs @@ -29,7 +29,7 @@ use super::support::prepared_digest; fn rank(bencher: Bencher) { let digest = prepared_digest(); - bencher.bench_local(|| black_box(&digest).rank(black_box(0.531_25))); + bencher.bench_local(|| black_box(&digest).rank(black_box(0.531_25)).unwrap()); } #[divan::bench(args = [0.0, 3.0, 3.5, 7.0])] @@ -37,14 +37,14 @@ fn rank_repeated_means(bencher: Bencher, value: f64) { let values = (0..100_000).map(|i| (i % 8) as f64).collect::>(); let digest = build_digest(&values); - bencher.bench_local(|| black_box(&digest).rank(black_box(value))); + bencher.bench_local(|| black_box(&digest).rank(black_box(value)).unwrap()); } #[divan::bench] fn quantile(bencher: Bencher) { let digest = prepared_digest(); - bencher.bench_local(|| black_box(&digest).quantile(black_box(0.531_25))); + bencher.bench_local(|| black_box(&digest).quantile(black_box(0.531_25)).unwrap()); } #[divan::bench] @@ -54,7 +54,7 @@ fn cdf_100(bencher: Bencher) { bencher .counter(ItemsCount::new(split_points.len())) - .bench_local(|| black_box(&digest).cdf(black_box(&split_points))); + .bench_local(|| black_box(&digest).cdf(black_box(&split_points)).unwrap()); } #[divan::bench(consts = [true, false], args = [2, 6, 100])] @@ -64,7 +64,7 @@ fn quantiles_sequential(bencher: Bencher, num_ranks: usize) bencher.counter(ItemsCount::new(num_ranks)).bench_local(|| { for &rank in black_box(&ranks) { - black_box(black_box(&digest).quantile(black_box(rank))); + black_box(black_box(&digest).quantile(black_box(rank)).unwrap()); } }); } @@ -76,7 +76,7 @@ fn quantiles_batch(bencher: Bencher, num_ranks: usize) { bencher .counter(ItemsCount::new(num_ranks)) - .bench_local(|| black_box(&digest).quantiles(black_box(&ranks))); + .bench_local(|| black_box(&digest).quantiles(black_box(&ranks)).unwrap()); } fn query_ranks(num_ranks: usize, sorted: bool) -> Vec { diff --git a/datasketches/src/kll/mod.rs b/datasketches/src/kll/mod.rs index 469bc7f1..59e0fd62 100644 --- a/datasketches/src/kll/mod.rs +++ b/datasketches/src/kll/mod.rs @@ -31,13 +31,17 @@ //! # Usage //! //! ```rust -//! # use datasketches::common::SearchCriteria; -//! # use datasketches::kll::KllSketch; -//! let mut sketch = KllSketch::::new(200).unwrap(); +//! use datasketches::common::SearchCriteria; +//! use datasketches::kll::KllSketch; +//! +//! let mut sketch = KllSketch::::new(200)?; //! sketch.update(1); //! sketch.update(2); -//! let q = sketch.quantile(0.5, SearchCriteria::Inclusive).unwrap(); +//! let Some(q) = sketch.quantile(0.5, SearchCriteria::Inclusive)? else { +//! return Ok(()); // No samples to query. +//! }; //! assert!((1..=2).contains(&q)); +//! # Ok::<(), datasketches::error::Error>(()) //! ``` mod capacity; diff --git a/datasketches/src/kll/sketch.rs b/datasketches/src/kll/sketch.rs index 9fb3a62a..891c4bbf 100644 --- a/datasketches/src/kll/sketch.rs +++ b/datasketches/src/kll/sketch.rs @@ -79,7 +79,8 @@ impl KllSketch { /// # Examples /// /// ``` - /// # use datasketches::kll::KllSketch; + /// use datasketches::kll::KllSketch; + /// /// let sketch = KllSketch::::new(200).unwrap(); /// assert_eq!(sketch.k(), 200); /// ``` @@ -123,11 +124,15 @@ impl KllSketch { } /// Returns the minimum item seen by the sketch. + /// + /// Returns `None` if the sketch is empty. pub fn min_item(&self) -> Option<&T> { self.min_item.as_ref() } /// Returns the maximum item seen by the sketch. + /// + /// Returns `None` if the sketch is empty. pub fn max_item(&self) -> Option<&T> { self.max_item.as_ref() } @@ -202,12 +207,10 @@ impl KllSketch { /// Returns the normalized rank of the given item. /// - /// # Errors - /// - /// Returns an error if the sketch is empty. - pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Result { + /// Returns `None` if the sketch is empty. + pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Option { if self.is_empty() { - return Err(Error::invalid_argument("cannot query an empty sketch")); + return None; } let inclusive = criteria == SearchCriteria::Inclusive; let mut weight = 0u64; @@ -222,70 +225,87 @@ impl KllSketch { .count() as u64; weight += count << level; } - Ok(weight as f64 / self.n as f64) + Some(weight as f64 / self.n as f64) } /// Returns the quantile for the given normalized rank. /// - /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. + /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. Returns + /// `Ok(None)` if the sketch is empty. /// /// # Errors /// - /// Returns an error if the sketch is empty or `rank` is outside `[0.0, 1.0]`. - pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result { - if self.is_empty() { - return Err(Error::invalid_argument("cannot query an empty sketch")); - } + /// Returns `InvalidArgument` if `rank` is NaN or outside `[0.0, 1.0]`, even if the sketch is + /// empty. + pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result, Error> { if !(0.0..=1.0).contains(&rank) { return Err(Error::invalid_argument(format!( "rank must be in [0.0, 1.0], got {rank}" ))); } + if self.is_empty() { + return Ok(None); + } if rank == 0.0 { - return Ok(self.min_item.as_ref().unwrap().clone()); + return Ok(Some(self.min_item.as_ref().unwrap().clone())); } if rank == 1.0 { - return Ok(self.max_item.as_ref().unwrap().clone()); + return Ok(Some(self.max_item.as_ref().unwrap().clone())); } self.sorted_view().quantile(rank, criteria) } /// Returns approximate quantiles for the given normalized ranks. /// - /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. + /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. Returns + /// `Ok(None)` if the sketch is empty. Otherwise, an empty `ranks` slice returns + /// `Ok(Some(vec![]))`. /// /// The sorted view is built once for the whole batch. /// /// # Errors /// - /// Returns an error if the sketch is empty or any rank is outside `[0.0, 1.0]`. - pub fn quantiles(&self, ranks: &[f64], criteria: SearchCriteria) -> Result, Error> { + /// Returns `InvalidArgument` if any rank is NaN or outside `[0.0, 1.0]`, even if the sketch is + /// empty. + pub fn quantiles( + &self, + ranks: &[f64], + criteria: SearchCriteria, + ) -> Result>, Error> { self.sorted_view().quantiles(ranks, criteria) } /// Returns the approximate CDF for the given split points. /// + /// Returns `Ok(None)` if the sketch is empty. Otherwise, an empty `split_points` slice returns + /// `Ok(Some(vec![1.0]))`. + /// /// # Errors /// - /// Returns an error if the sketch is empty or the split points are not unique and strictly - /// increasing. - pub fn cdf(&self, split_points: &[T], criteria: SearchCriteria) -> Result, Error> { - if self.is_empty() { - return Err(Error::invalid_argument("cannot query an empty sketch")); - } + /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the + /// sketch is empty. + pub fn cdf( + &self, + split_points: &[T], + criteria: SearchCriteria, + ) -> Result>, Error> { self.sorted_view().cdf(split_points, criteria) } /// Returns the approximate PMF for the given split points. /// + /// Returns `Ok(None)` if the sketch is empty. Otherwise, an empty `split_points` slice returns + /// `Ok(Some(vec![1.0]))`. + /// /// # Errors /// - /// Returns an error if the sketch is empty or the split points are not unique and strictly - /// increasing. - pub fn pmf(&self, split_points: &[T], criteria: SearchCriteria) -> Result, Error> { - if self.is_empty() { - return Err(Error::invalid_argument("cannot query an empty sketch")); - } + /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the + /// sketch is empty. + pub fn pmf( + &self, + split_points: &[T], + criteria: SearchCriteria, + ) -> Result>, Error> { self.sorted_view().pmf(split_points, criteria) } diff --git a/datasketches/src/kll/sorted_view.rs b/datasketches/src/kll/sorted_view.rs index d11f45be..35eba5d2 100644 --- a/datasketches/src/kll/sorted_view.rs +++ b/datasketches/src/kll/sorted_view.rs @@ -70,12 +70,10 @@ impl SortedView { /// Returns the approximate normalized rank of `item`. /// - /// # Errors - /// - /// Returns an error if the view is empty. - pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Result { + /// Returns `None` if the view is empty. + pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Option { if self.is_empty() { - return Err(Error::invalid_argument("cannot query an empty view")); + return None; } let index = if criteria == SearchCriteria::Inclusive { upper_bound(&self.entries, item) @@ -84,34 +82,36 @@ impl SortedView { }; if index == 0 { - return Ok(0.0); + return Some(0.0); } - Ok(self.entries[index - 1].cumulative_weight as f64 / self.total_weight as f64) + Some(self.entries[index - 1].cumulative_weight as f64 / self.total_weight as f64) } /// Returns the approximate quantile for `rank`. /// - /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. + /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. Returns + /// `Ok(None)` if the view is empty. /// /// # Errors /// - /// Returns an error if the view is empty or `rank` is outside `[0.0, 1.0]`. - pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result { - if self.is_empty() { - return Err(Error::invalid_argument("cannot query an empty view")); - } + /// Returns `InvalidArgument` if `rank` is NaN or outside `[0.0, 1.0]`, even if the view is + /// empty. + pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result, Error> { if !(0.0..=1.0).contains(&rank) { return Err(Error::invalid_argument(format!( "rank must be in [0.0, 1.0], got {rank}" ))); } + if self.is_empty() { + return Ok(None); + } // Compaction may discard extrema, and large weights can lose precision in f64. if rank == 0.0 { - return Ok(self.min_item.as_ref().unwrap().clone()); + return Ok(Some(self.min_item.as_ref().unwrap().clone())); } if rank == 1.0 { - return Ok(self.max_item.as_ref().unwrap().clone()); + return Ok(Some(self.max_item.as_ref().unwrap().clone())); } let weight = if criteria == SearchCriteria::Inclusive { @@ -125,55 +125,93 @@ impl SortedView { upper_bound_by_weight(&self.entries, weight) }; - Ok(self.entries[index.min(self.entries.len() - 1)].item.clone()) + Ok(Some( + self.entries[index.min(self.entries.len() - 1)].item.clone(), + )) } /// Returns approximate quantiles for all `ranks`. /// - /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. + /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. Returns + /// `Ok(None)` if the view is empty. Otherwise, an empty `ranks` slice returns + /// `Ok(Some(vec![]))`. /// /// # Errors /// - /// Returns an error if the view is empty or any rank is outside `[0.0, 1.0]`. - pub fn quantiles(&self, ranks: &[f64], criteria: SearchCriteria) -> Result, Error> { + /// Returns `InvalidArgument` if any rank is NaN or outside `[0.0, 1.0]`, even if the view is + /// empty. + pub fn quantiles( + &self, + ranks: &[f64], + criteria: SearchCriteria, + ) -> Result>, Error> { + for &rank in ranks { + if !(0.0..=1.0).contains(&rank) { + return Err(Error::invalid_argument(format!( + "rank must be in [0.0, 1.0], got {rank}" + ))); + } + } if self.is_empty() { - return Err(Error::invalid_argument("cannot query an empty view")); + return Ok(None); } - ranks + let quantiles = ranks .iter() - .map(|&rank| self.quantile(rank, criteria)) - .collect() + .map(|&rank| { + self.quantile(rank, criteria) + .map(|quantile| quantile.expect("checked non-empty view")) + }) + .collect::>()?; + Ok(Some(quantiles)) } /// Returns the approximate cumulative distribution over `split_points`. /// + /// Returns `Ok(None)` if the view is empty. Otherwise, an empty `split_points` slice returns + /// `Ok(Some(vec![1.0]))`. + /// /// # Errors /// - /// Returns an error if the view is empty or the split points are invalid. - pub fn cdf(&self, split_points: &[T], criteria: SearchCriteria) -> Result, Error> { + /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the view + /// is empty. + pub fn cdf( + &self, + split_points: &[T], + criteria: SearchCriteria, + ) -> Result>, Error> { + check_split_points(split_points)?; if self.is_empty() { - return Err(Error::invalid_argument("cannot query an empty view")); + return Ok(None); } - check_split_points(split_points)?; let mut ranks = Vec::with_capacity(split_points.len() + 1); for item in split_points { - ranks.push(self.rank(item, criteria)?); + ranks.push(self.rank(item, criteria).expect("checked non-empty view")); } ranks.push(1.0); - Ok(ranks) + Ok(Some(ranks)) } /// Returns the approximate probability mass over `split_points`. /// + /// Returns `Ok(None)` if the view is empty. Otherwise, an empty `split_points` slice returns + /// `Ok(Some(vec![1.0]))`. + /// /// # Errors /// - /// Returns an error if the view is empty or the split points are invalid. - pub fn pmf(&self, split_points: &[T], criteria: SearchCriteria) -> Result, Error> { - let mut buckets = self.cdf(split_points, criteria)?; + /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the view + /// is empty. + pub fn pmf( + &self, + split_points: &[T], + criteria: SearchCriteria, + ) -> Result>, Error> { + let Some(mut buckets) = self.cdf(split_points, criteria)? else { + return Ok(None); + }; for index in (1..buckets.len()).rev() { buckets[index] -= buckets[index - 1]; } - Ok(buckets) + Ok(Some(buckets)) } } diff --git a/datasketches/src/req/mod.rs b/datasketches/src/req/mod.rs index ce5a65fe..40b787c0 100644 --- a/datasketches/src/req/mod.rs +++ b/datasketches/src/req/mod.rs @@ -44,7 +44,9 @@ //! sketch.update(ReqFloat::::new(value)?); //! } //! -//! let median = sketch.quantile(0.5, SearchCriteria::Inclusive)?; +//! let Some(median) = sketch.quantile(0.5, SearchCriteria::Inclusive)? else { +//! return Ok(()); // No samples to query. +//! }; //! assert_eq!(median.into_inner(), 2.0); //! # Ok::<(), datasketches::error::Error>(()) //! ``` diff --git a/datasketches/src/req/sketch.rs b/datasketches/src/req/sketch.rs index 77a3284a..446906f1 100644 --- a/datasketches/src/req/sketch.rs +++ b/datasketches/src/req/sketch.rs @@ -176,16 +176,13 @@ where /// Returns the approximate rank of `item` in `[0.0, 1.0]`. /// - /// Computed directly from the retained items in a single `O(retained)` pass, - /// without building a sorted view. The result is identical to - /// [`SortedView::rank`] on [`Self::sorted_view`]. + /// Computed directly from the retained items in a single `O(retained)` pass, without building + /// a sorted view. The result is identical to [`SortedView::rank`] on [`Self::sorted_view`]. /// - /// # Errors - /// - /// Returns an error if the sketch is empty. - pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Result { + /// Returns `None` if the sketch is empty. + pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Option { if self.is_empty() { - return Err(Error::invalid_argument("sketch is empty")); + return None; } let inclusive = matches!(criteria, SearchCriteria::Inclusive); let weight: u64 = self @@ -193,50 +190,56 @@ where .iter() .map(|c| c.count_below(item, inclusive) as u64 * c.weight()) .sum(); - Ok(weight as f64 / self.n as f64) + Some(weight as f64 / self.n as f64) } /// Returns the approximate quantile at the given normalized rank. /// - /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. + /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. Returns + /// `Ok(None)` if the sketch is empty. /// /// For ranks strictly between `0.0` and `1.0`, builds a transient [`SortedView`]. /// For repeated queries at those ranks, take one snapshot with [`Self::sorted_view`]. /// /// # Errors /// - /// Returns an error if the sketch is empty or `rank` is outside `[0.0, 1.0]`. - pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result { - if self.is_empty() { - return Err(Error::invalid_argument("sketch is empty")); - } + /// Returns `InvalidArgument` if `rank` is NaN or outside `[0.0, 1.0]`, even if the sketch is + /// empty. + pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result, Error> { if !(0.0..=1.0).contains(&rank) { return Err(Error::invalid_argument(format!( "rank {rank} must be in [0, 1]" ))); } + if self.is_empty() { + return Ok(None); + } if rank == 0.0 { - return Ok(self.min_item.as_ref().unwrap().clone()); + return Ok(Some(self.min_item.as_ref().unwrap().clone())); } if rank == 1.0 { - return Ok(self.max_item.as_ref().unwrap().clone()); + return Ok(Some(self.max_item.as_ref().unwrap().clone())); } self.sorted_view().quantile(rank, criteria) } /// Returns approximate quantiles for the given normalized ranks. /// - /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. + /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. Returns + /// `Ok(None)` if the sketch is empty. Otherwise, an empty `ranks` slice returns + /// `Ok(Some(vec![]))`. /// /// The sorted view is built once and shared across all ranks. /// /// # Errors /// - /// Returns an error if the sketch is empty or any rank is outside `[0.0, 1.0]`. - pub fn quantiles(&self, ranks: &[f64], criteria: SearchCriteria) -> Result, Error> { - if self.is_empty() { - return Err(Error::invalid_argument("sketch is empty")); - } + /// Returns `InvalidArgument` if any rank is NaN or outside `[0.0, 1.0]`, even if the sketch is + /// empty. + pub fn quantiles( + &self, + ranks: &[f64], + criteria: SearchCriteria, + ) -> Result>, Error> { // Reject invalid ranks before paying for the view build. for &r in ranks { if !(0.0..=1.0).contains(&r) { @@ -245,45 +248,63 @@ where ))); } } + if self.is_empty() { + return Ok(None); + } let view = self.sorted_view(); - ranks.iter().map(|&r| view.quantile(r, criteria)).collect() + let quantiles = ranks + .iter() + .map(|&r| { + view.quantile(r, criteria) + .map(|quantile| quantile.expect("checked non-empty view")) + }) + .collect::>()?; + Ok(Some(quantiles)) } /// Returns the Probability Mass Function over the given split points. /// + /// Returns `Ok(None)` if the sketch is empty. Otherwise, an empty `split_points` slice returns + /// `Ok(Some(vec![1.0]))`. + /// /// # Errors /// - /// Returns an error if the sketch is empty or the split points are not strictly increasing. - pub fn pmf(&self, split_points: &[T], criteria: SearchCriteria) -> Result, Error> { - if self.is_empty() { - return Err(Error::invalid_argument("sketch is empty")); - } + /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the + /// sketch is empty. + pub fn pmf( + &self, + split_points: &[T], + criteria: SearchCriteria, + ) -> Result>, Error> { self.sorted_view().pmf(split_points, criteria) } /// Returns the Cumulative Distribution Function over the given split points. /// + /// Returns `Ok(None)` if the sketch is empty. Otherwise, an empty `split_points` slice returns + /// `Ok(Some(vec![1.0]))`. + /// /// # Errors /// - /// Returns an error if the sketch is empty or the split points are not strictly increasing. - pub fn cdf(&self, split_points: &[T], criteria: SearchCriteria) -> Result, Error> { - if self.is_empty() { - return Err(Error::invalid_argument("sketch is empty")); - } + /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the + /// sketch is empty. + pub fn cdf( + &self, + split_points: &[T], + criteria: SearchCriteria, + ) -> Result>, Error> { self.sorted_view().cdf(split_points, criteria) } /// Returns an owned, sorted snapshot of the sketch's current state. /// - /// An empty sketch yields an empty view; queries on it return an error. The - /// view is independent of the sketch — it can be queried (and sent to other - /// threads) while the sketch keeps receiving updates, and it keeps answering - /// from the state it was taken at. + /// An empty sketch yields an empty view. The view can be queried or sent to other threads + /// while the sketch receives updates, preserving the state captured when it was created. /// /// Building the view costs `O(retained · log retained)`; rank and quantile queries /// then take `O(log retained)`. Prefer reusing one view for repeated quantile - /// queries at ranks strictly between `0.0` and `1.0`, or repeated - /// [`Self::pmf`]/[`Self::cdf`] queries. + /// queries at ranks strictly between `0.0` and `1.0`, or repeated [`Self::pmf`]/[`Self::cdf`] + /// queries. pub fn sorted_view(&self) -> SortedView { let mut weighted_items = Vec::with_capacity(self.num_retained as usize); for compactor in &self.compactors { @@ -371,6 +392,8 @@ where } /// Returns the lower bound for the normalized `rank` at the requested confidence level. + /// + /// In exact mode, including an empty sketch, the bound equals `rank`. pub fn rank_lower_bound(&self, rank: f64, num_std_dev: NumStdDev) -> f64 { self.compute_rank_lower_bound( self.k, @@ -383,6 +406,8 @@ where } /// Returns the upper bound for the normalized `rank` at the requested confidence level. + /// + /// In exact mode, including an empty sketch, the bound equals `rank`. pub fn rank_upper_bound(&self, rank: f64, num_std_dev: NumStdDev) -> f64 { self.compute_rank_upper_bound( self.k, diff --git a/datasketches/src/req/sorted_view.rs b/datasketches/src/req/sorted_view.rs index ec62f873..78c4bc65 100644 --- a/datasketches/src/req/sorted_view.rs +++ b/datasketches/src/req/sorted_view.rs @@ -22,12 +22,11 @@ use crate::error::Error; /// An owned, sorted snapshot of a [`ReqSketch`](crate::req::ReqSketch). /// -/// Obtain one with [`ReqSketch::sorted_view`](crate::req::ReqSketch::sorted_view). -/// The view is independent of the sketch: it can be queried (and sent to other -/// threads) while the sketch keeps receiving updates, and it keeps answering -/// from the state it was taken at. Building it costs `O(retained · log retained)`; -/// each subsequent query is `O(log retained)`, so it is the right tool for -/// repeated quantile/rank queries. +/// Obtain one with [`ReqSketch::sorted_view`](crate::req::ReqSketch::sorted_view). The view is +/// independent of the sketch: it can be queried (and sent to other threads) while the sketch keeps +/// receiving updates, and it keeps answering from the state it was taken at. Building it costs +/// `O(retained · log retained)`; each subsequent query is `O(log retained)`, so it is the right +/// tool for repeated quantile/rank queries. #[derive(Debug, Clone)] pub struct SortedView { items: Vec, @@ -93,48 +92,47 @@ where /// Returns the approximate normalized rank of `item` in `[0.0, 1.0]`. /// - /// # Errors - /// - /// Returns an error if the view is empty. - pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Result { + /// Returns `None` if the view is empty. + pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Option { if self.is_empty() { - return Err(Error::invalid_argument("sketch is empty")); + return None; } let pos = match criteria { SearchCriteria::Inclusive => self.items.partition_point(|x| x <= item), SearchCriteria::Exclusive => self.items.partition_point(|x| x < item), }; if pos == 0 { - Ok(0.0) + Some(0.0) } else { - Ok(self.cumulative_weights[pos - 1] as f64 / self.total_weight as f64) + Some(self.cumulative_weights[pos - 1] as f64 / self.total_weight as f64) } } /// Returns the approximate quantile at the given normalized rank. /// - /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. + /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. Returns + /// `Ok(None)` if the view is empty. /// /// # Errors /// - /// Returns an error if the view is empty or `rank` is outside `[0.0, 1.0]`. - pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result { - if self.is_empty() { - return Err(Error::invalid_argument("sketch is empty")); - } - + /// Returns `InvalidArgument` if `rank` is NaN or outside `[0.0, 1.0]`, even if the view is + /// empty. + pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result, Error> { if !(0.0..=1.0).contains(&rank) { return Err(Error::invalid_argument(format!( "rank {rank} must be in [0, 1]" ))); } + if self.is_empty() { + return Ok(None); + } // Compaction may discard extrema, and large weights can lose precision in f64. if rank == 0.0 { - return Ok(self.min_item.as_ref().unwrap().clone()); + return Ok(Some(self.min_item.as_ref().unwrap().clone())); } if rank == 1.0 { - return Ok(self.max_item.as_ref().unwrap().clone()); + return Ok(Some(self.max_item.as_ref().unwrap().clone())); } let target_weight = match criteria { @@ -151,44 +149,61 @@ where .partition_point(|&w| w <= target_weight), }; - Ok(self.items[index.min(self.items.len() - 1)].clone()) + Ok(Some(self.items[index.min(self.items.len() - 1)].clone())) } /// Returns the probability mass function (PMF) over the given split points. /// - /// The result contains one more value than `split_points`. + /// The result contains one more value than `split_points`. Returns `Ok(None)` if the view is + /// empty. Otherwise, an empty `split_points` slice returns `Ok(Some(vec![1.0]))`. /// /// # Errors /// - /// Returns an error if the view is empty or the split points are not strictly increasing. - pub fn pmf(&self, split_points: &[T], criteria: SearchCriteria) -> Result, Error> { - let mut result = self.cdf(split_points, criteria)?; + /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the view + /// is empty. + pub fn pmf( + &self, + split_points: &[T], + criteria: SearchCriteria, + ) -> Result>, Error> { + let Some(mut result) = self.cdf(split_points, criteria)? else { + return Ok(None); + }; for index in (1..result.len()).rev() { result[index] -= result[index - 1]; } - Ok(result) + Ok(Some(result)) } /// Returns the cumulative distribution function (CDF) over the given split points. /// - /// The result contains one more value than `split_points` and ends at `1.0`. + /// The result contains one more value than `split_points` and ends at `1.0`. Returns + /// `Ok(None)` if the view is empty. Otherwise, an empty `split_points` slice returns + /// `Ok(Some(vec![1.0]))`. /// /// # Errors /// - /// Returns an error if the view is empty or the split points are not strictly increasing. - pub fn cdf(&self, split_points: &[T], criteria: SearchCriteria) -> Result, Error> { + /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the view + /// is empty. + pub fn cdf( + &self, + split_points: &[T], + criteria: SearchCriteria, + ) -> Result>, Error> { + self.validate_split_points(split_points)?; if self.is_empty() { - return Err(Error::invalid_argument("sketch is empty")); + return Ok(None); } - self.validate_split_points(split_points)?; - let mut result = Vec::with_capacity(split_points.len() + 1); for split_point in split_points { - result.push(self.rank(split_point, criteria)?); + result.push( + self.rank(split_point, criteria) + .expect("checked non-empty view"), + ); } result.push(1.0); - Ok(result) + Ok(Some(result)) } fn validate_split_points(&self, split_points: &[T]) -> Result<(), Error> { @@ -205,8 +220,6 @@ where mod tests { use googletest::assert_that; use googletest::prelude::all; - use googletest::prelude::anything; - use googletest::prelude::err; use googletest::prelude::ge; use googletest::prelude::le; use googletest::prelude::near; @@ -227,21 +240,38 @@ mod tests { } #[test] - fn test_rank_queries() -> Result<(), Error> { + fn test_rank_queries() { let view = create_test_view(); // Test exact matches - assert_that!(view.rank(&1, SearchCriteria::Inclusive)?, near(0.2, 1e-10)); - assert_that!(view.rank(&1, SearchCriteria::Exclusive)?, near(0.0, 1e-10)); + assert_that!( + view.rank(&1, SearchCriteria::Inclusive).unwrap(), + near(0.2, 1e-10) + ); + assert_that!( + view.rank(&1, SearchCriteria::Exclusive).unwrap(), + near(0.0, 1e-10) + ); // Test values between items - assert_that!(view.rank(&2, SearchCriteria::Inclusive)?, near(0.2, 1e-10)); - assert_that!(view.rank(&6, SearchCriteria::Inclusive)?, near(0.6, 1e-10)); + assert_that!( + view.rank(&2, SearchCriteria::Inclusive).unwrap(), + near(0.2, 1e-10) + ); + assert_that!( + view.rank(&6, SearchCriteria::Inclusive).unwrap(), + near(0.6, 1e-10) + ); // Test edge cases - assert_that!(view.rank(&0, SearchCriteria::Inclusive)?, near(0.0, 1e-10)); - assert_that!(view.rank(&10, SearchCriteria::Inclusive)?, near(1.0, 1e-10)); - Ok(()) + assert_that!( + view.rank(&0, SearchCriteria::Inclusive).unwrap(), + near(0.0, 1e-10) + ); + assert_that!( + view.rank(&10, SearchCriteria::Inclusive).unwrap(), + near(1.0, 1e-10) + ); } #[test] @@ -249,16 +279,16 @@ mod tests { let view = create_test_view(); // Test edge cases - assert_eq!(view.quantile(0.0, SearchCriteria::Inclusive)?, 1); - assert_eq!(view.quantile(1.0, SearchCriteria::Inclusive)?, 9); + assert_eq!(view.quantile(0.0, SearchCriteria::Inclusive)?, Some(1)); + assert_eq!(view.quantile(1.0, SearchCriteria::Inclusive)?, Some(9)); // Test middle values - let median = view.quantile(0.5, SearchCriteria::Inclusive)?; + let median = view.quantile(0.5, SearchCriteria::Inclusive)?.unwrap(); assert_that!(median, all!(ge(3), le(7))); // Should be around the middle (values are 1,3,5,7,9) // Test various ranks - let q25 = view.quantile(0.25, SearchCriteria::Inclusive)?; - let q75 = view.quantile(0.75, SearchCriteria::Inclusive)?; + let q25 = view.quantile(0.25, SearchCriteria::Inclusive)?.unwrap(); + let q75 = view.quantile(0.75, SearchCriteria::Inclusive)?.unwrap(); assert_that!(q25, le(median)); assert_that!(median, le(q75)); Ok(()) @@ -269,7 +299,7 @@ mod tests { let view = create_test_view(); let split_points = vec![3, 7]; - let pmf = view.pmf(&split_points, SearchCriteria::Inclusive)?; + let pmf = view.pmf(&split_points, SearchCriteria::Inclusive)?.unwrap(); assert_eq!(pmf.len(), 3); // 2 split points create 3 intervals // Sum should be approximately 1.0 @@ -283,7 +313,7 @@ mod tests { let view = create_test_view(); let split_points = vec![3, 7]; - let cdf = view.cdf(&split_points, SearchCriteria::Inclusive)?; + let cdf = view.cdf(&split_points, SearchCriteria::Inclusive)?.unwrap(); assert_eq!(cdf.len(), 3); // CDF should be monotonically increasing @@ -303,11 +333,7 @@ mod tests { assert_eq!(view.len(), 0); assert_eq!(view.total_weight(), 0); - // Operations on empty view should return errors - assert_that!(view.rank(&5, SearchCriteria::Inclusive), err(anything())); - assert_that!( - view.quantile(0.5, SearchCriteria::Inclusive), - err(anything()) - ); + assert_eq!(view.rank(&5, SearchCriteria::Inclusive), None); + assert_eq!(view.quantile(0.5, SearchCriteria::Inclusive).unwrap(), None); } } diff --git a/datasketches/src/tdigest/mod.rs b/datasketches/src/tdigest/mod.rs index 0d466187..8c5818b4 100644 --- a/datasketches/src/tdigest/mod.rs +++ b/datasketches/src/tdigest/mod.rs @@ -56,9 +56,13 @@ //! let mut sketch = TDigestMut::new(100).unwrap(); //! sketch.update(1.0); //! sketch.update(2.0); -//! let median = sketch.quantile(0.5).unwrap(); +//! let Some(median) = sketch.quantile(0.5)? else { +//! return Ok(()); // No samples to query. +//! }; +//! assert!((1.0..=2.0).contains(&median)); //! let frozen = sketch.freeze(); -//! assert!(frozen.rank(2.0).is_some()); +//! assert!(frozen.rank(2.0)?.is_some()); +//! # Ok::<(), datasketches::error::Error>(()) //! ``` mod serialization; diff --git a/datasketches/src/tdigest/sketch.rs b/datasketches/src/tdigest/sketch.rs index 810c86be..d7599af3 100644 --- a/datasketches/src/tdigest/sketch.rs +++ b/datasketches/src/tdigest/sketch.rs @@ -403,10 +403,12 @@ impl TDigestMut { /// Returns the cumulative distribution approximation described by [`TDigest::cdf`]. /// - /// # Panics + /// Returns `Ok(None)` if this t-digest is empty. + /// + /// # Errors /// - /// Panics if `split_points` is not unique, not monotonically increasing, or contains `NaN` - /// values. + /// Returns `InvalidArgument` if `split_points` contains NaN or is not strictly increasing, even + /// if this t-digest is empty. /// /// # Examples /// @@ -417,25 +419,27 @@ impl TDigestMut { /// for value in [1.0, 2.0, 3.0] { /// sketch.update(value); /// } - /// let cdf = sketch.cdf(&[1.5]).unwrap(); + /// let cdf = sketch.cdf(&[1.5]).unwrap().unwrap(); /// assert_eq!(cdf.len(), 2); /// ``` - pub fn cdf(&mut self, split_points: &[f64]) -> Option> { - check_split_points(split_points); + pub fn cdf(&mut self, split_points: &[f64]) -> Result>, Error> { + check_split_points(split_points)?; if self.is_empty() { - return None; + return Ok(None); } - self.view().cdf(split_points) + Ok(self.view().cdf(split_points)) } /// Returns the probability mass approximation described by [`TDigest::pmf`]. /// - /// # Panics + /// Returns `Ok(None)` if this t-digest is empty. /// - /// Panics if `split_points` is not unique, not monotonically increasing, or contains `NaN` - /// values. + /// # Errors + /// + /// Returns `InvalidArgument` if `split_points` contains NaN or is not strictly increasing, even + /// if this t-digest is empty. /// /// # Examples /// @@ -446,24 +450,26 @@ impl TDigestMut { /// for value in [1.0, 2.0, 3.0] { /// sketch.update(value); /// } - /// let pmf = sketch.pmf(&[1.5]).unwrap(); + /// let pmf = sketch.pmf(&[1.5]).unwrap().unwrap(); /// assert_eq!(pmf.len(), 2); /// ``` - pub fn pmf(&mut self, split_points: &[f64]) -> Option> { - check_split_points(split_points); + pub fn pmf(&mut self, split_points: &[f64]) -> Result>, Error> { + check_split_points(split_points)?; if self.is_empty() { - return None; + return Ok(None); } - self.view().pmf(split_points) + Ok(self.view().pmf(split_points)) } /// Returns the normalized rank described by [`TDigest::rank`]. /// - /// # Panics + /// Returns `Ok(None)` if this t-digest is empty. + /// + /// # Errors /// - /// Panics if `value` is `NaN`. + /// Returns `InvalidArgument` if `value` is NaN, even if this t-digest is empty. /// /// # Examples /// @@ -474,33 +480,38 @@ impl TDigestMut { /// for value in [1.0, 2.0, 3.0] { /// sketch.update(value); /// } - /// let rank = sketch.rank(2.0).unwrap(); + /// let rank = sketch.rank(2.0).unwrap().unwrap(); /// assert!((0.0..=1.0).contains(&rank)); /// ``` - pub fn rank(&mut self, value: f64) -> Option { - assert!(!value.is_nan(), "value must not be NaN"); + pub fn rank(&mut self, value: f64) -> Result, Error> { + if value.is_nan() { + return Err(Error::invalid_argument("value must not be NaN")); + } if self.is_empty() { - return None; + return Ok(None); } if value < self.min { - return Some(0.0); + return Ok(Some(0.0)); } if value > self.max { - return Some(1.0); + return Ok(Some(1.0)); } if self.min == self.max { - return Some(0.5); + return Ok(Some(0.5)); } - self.view().rank(value) + Ok(self.view().rank(value)) } /// Returns the quantile described by [`TDigest::quantile`]. /// - /// # Panics + /// Returns `Ok(None)` if this t-digest is empty. /// - /// Panics if `rank` is outside `[0.0, 1.0]`. + /// # Errors + /// + /// Returns `InvalidArgument` if `rank` is NaN or outside `[0.0, 1.0]`, even if this t-digest is + /// empty. /// /// # Examples /// @@ -511,35 +522,36 @@ impl TDigestMut { /// for value in [1.0, 2.0, 3.0] { /// sketch.update(value); /// } - /// let median = sketch.quantile(0.5).unwrap(); + /// let median = sketch.quantile(0.5).unwrap().unwrap(); /// assert!((1.0..=3.0).contains(&median)); /// ``` - pub fn quantile(&mut self, rank: f64) -> Option { - assert!( - (0.0..=1.0).contains(&rank), - "rank must be in [0.0, 1.0]; got {rank}" - ); + pub fn quantile(&mut self, rank: f64) -> Result, Error> { + check_rank(rank)?; if self.is_empty() { - return None; + return Ok(None); } - self.view().quantile(rank) + Ok(self.view().quantile(rank)) } /// Returns the quantiles described by [`TDigest::quantiles`]. /// - /// # Panics + /// Returns `Ok(None)` if this t-digest is empty. Otherwise, an empty `ranks` slice returns + /// `Ok(Some(vec![]))`. /// - /// Panics if any rank is outside `[0.0, 1.0]`. - pub fn quantiles(&mut self, ranks: &[f64]) -> Option> { - check_ranks(ranks); + /// # Errors + /// + /// Returns `InvalidArgument` if any rank is NaN or outside `[0.0, 1.0]`, even if this t-digest + /// is empty. + pub fn quantiles(&mut self, ranks: &[f64]) -> Result>, Error> { + check_ranks(ranks)?; if self.is_empty() { - return None; + return Ok(None); } - self.view().quantiles(ranks) + Ok(self.view().quantiles(ranks)) } /// Serializes this mutable t-digest to bytes. @@ -1427,14 +1439,15 @@ impl TDigest { /// stream given the split points. The value at array position j of the returned CDF array /// is the sum of the returned values in positions 0 through j of the returned PMF array. /// This can be viewed as array of ranks of the given split points plus one more value that - /// is always 1. An empty `split_points` slice returns the single value `[1.0]`. + /// is always 1. For a non-empty t-digest, an empty `split_points` slice returns + /// `Ok(Some(vec![1.0]))`. /// - /// Returns `None` if this t-digest is empty. + /// Returns `Ok(None)` if this t-digest is empty. /// - /// # Panics + /// # Errors /// - /// Panics if `split_points` is not unique, not monotonically increasing, or contains `NaN` - /// values. + /// Returns `InvalidArgument` if `split_points` contains NaN or is not strictly increasing, even + /// if this t-digest is empty. /// /// # Examples /// @@ -1446,11 +1459,12 @@ impl TDigest { /// sketch.update(value); /// } /// let digest = sketch.freeze(); - /// let cdf = digest.cdf(&[1.5]).unwrap(); + /// let cdf = digest.cdf(&[1.5]).unwrap().unwrap(); /// assert_eq!(cdf.len(), 2); /// ``` - pub fn cdf(&self, split_points: &[f64]) -> Option> { - self.view().cdf(split_points) + pub fn cdf(&self, split_points: &[f64]) -> Result>, Error> { + check_split_points(split_points)?; + Ok(self.view().cdf(split_points)) } /// Returns an approximation to the Probability Mass Function (PMF) of the input stream @@ -1465,14 +1479,14 @@ impl TDigest { /// /// An array of m+1 doubles each of which is an approximation to the fraction of the input /// stream values (the mass) that fall into one of those intervals. - /// An empty `split_points` slice returns the single value `[1.0]`. + /// For a non-empty t-digest, an empty `split_points` slice returns `Ok(Some(vec![1.0]))`. /// - /// Returns `None` if this t-digest is empty. + /// Returns `Ok(None)` if this t-digest is empty. /// - /// # Panics + /// # Errors /// - /// Panics if `split_points` is not unique, not monotonically increasing, or contains `NaN` - /// values. + /// Returns `InvalidArgument` if `split_points` contains NaN or is not strictly increasing, even + /// if this t-digest is empty. /// /// # Examples /// @@ -1484,20 +1498,21 @@ impl TDigest { /// sketch.update(value); /// } /// let digest = sketch.freeze(); - /// let pmf = digest.pmf(&[1.5]).unwrap(); + /// let pmf = digest.pmf(&[1.5]).unwrap().unwrap(); /// assert_eq!(pmf.len(), 2); /// ``` - pub fn pmf(&self, split_points: &[f64]) -> Option> { - self.view().pmf(split_points) + pub fn pmf(&self, split_points: &[f64]) -> Result>, Error> { + check_split_points(split_points)?; + Ok(self.view().pmf(split_points)) } - /// Computes the approximate normalized rank in `[0.0, 1.0]` of the given value. + /// Returns the approximate normalized rank in `[0.0, 1.0]` of the given value. /// - /// Returns `None` if this t-digest is empty. + /// Returns `Ok(None)` if this t-digest is empty. /// - /// # Panics + /// # Errors /// - /// Panics if the value is `NaN`. + /// Returns `InvalidArgument` if `value` is NaN, even if this t-digest is empty. /// /// # Examples /// @@ -1509,23 +1524,26 @@ impl TDigest { /// sketch.update(value); /// } /// let digest = sketch.freeze(); - /// let rank = digest.rank(2.0).unwrap(); + /// let rank = digest.rank(2.0).unwrap().unwrap(); /// assert!((0.0..=1.0).contains(&rank)); /// ``` - pub fn rank(&self, value: f64) -> Option { - assert!(!value.is_nan(), "value must not be NaN"); - self.view().rank(value) + pub fn rank(&self, value: f64) -> Result, Error> { + if value.is_nan() { + return Err(Error::invalid_argument("value must not be NaN")); + } + Ok(self.view().rank(value)) } - /// Computes the approximate quantile for the given normalized rank. + /// Returns the approximate quantile for the given normalized rank. /// /// Ranks `0.0` and `1.0` return the stored minimum and maximum, respectively. /// - /// Returns `None` if this t-digest is empty. + /// Returns `Ok(None)` if this t-digest is empty. /// - /// # Panics + /// # Errors /// - /// Panics if `rank` is outside `[0.0, 1.0]`. + /// Returns `InvalidArgument` if `rank` is NaN or outside `[0.0, 1.0]`, even if this t-digest is + /// empty. /// /// # Examples /// @@ -1537,24 +1555,26 @@ impl TDigest { /// sketch.update(value); /// } /// let digest = sketch.freeze(); - /// let q = digest.quantile(0.5).unwrap(); + /// let q = digest.quantile(0.5).unwrap().unwrap(); /// assert!((1.0..=3.0).contains(&q)); /// ``` - pub fn quantile(&self, rank: f64) -> Option { - assert!((0.0..=1.0).contains(&rank), "rank must be in [0.0, 1.0]"); - self.view().quantile(rank) + pub fn quantile(&self, rank: f64) -> Result, Error> { + check_rank(rank)?; + Ok(self.view().quantile(rank)) } - /// Computes approximate quantiles for the given normalized ranks. + /// Returns approximate quantiles for the given normalized ranks. /// /// Ranks in nondecreasing order are answered with one centroid scan. Ranks in any other order /// are accepted and results are returned in the same order as the input. /// - /// Returns `None` if this t-digest is empty. + /// Returns `Ok(None)` if this t-digest is empty. Otherwise, an empty `ranks` slice returns + /// `Ok(Some(vec![]))`. /// - /// # Panics + /// # Errors /// - /// Panics if any rank is outside `[0.0, 1.0]`. + /// Returns `InvalidArgument` if any rank is NaN or outside `[0.0, 1.0]`, even if this t-digest + /// is empty. /// /// # Examples /// @@ -1566,12 +1586,12 @@ impl TDigest { /// sketch.update(value); /// } /// let digest = sketch.freeze(); - /// let quantiles = digest.quantiles(&[0.25, 0.5, 0.75]).unwrap(); + /// let quantiles = digest.quantiles(&[0.25, 0.5, 0.75]).unwrap().unwrap(); /// assert_eq!(quantiles.len(), 3); /// ``` - pub fn quantiles(&self, ranks: &[f64]) -> Option> { - check_ranks(ranks); - self.view().quantiles(ranks) + pub fn quantiles(&self, ranks: &[f64]) -> Result>, Error> { + check_ranks(ranks)?; + Ok(self.view().quantiles(ranks)) } /// Converts this immutable t-digest into a mutable one. @@ -1622,8 +1642,6 @@ impl TDigestView<'_> { } fn cdf(&self, split_points: &[f64]) -> Option> { - check_split_points(split_points); - if self.centroids.is_empty() { return None; } @@ -1896,22 +1914,34 @@ impl TDigestView<'_> { /// Checks the sequential validity of the given array of double values. /// They must be unique, monotonically increasing and not NaN. -#[track_caller] -fn check_split_points(split_points: &[f64]) { +fn check_split_points(split_points: &[f64]) -> Result<(), Error> { if split_points.iter().any(|split_point| split_point.is_nan()) { - panic!("split_points must not contain NaN values: {split_points:?}"); + return Err(Error::invalid_argument( + "split_points must not contain NaN values", + )); } if !split_points.windows(2).all(|pair| pair[0] < pair[1]) { - panic!("split_points must be unique and monotonically increasing: {split_points:?}"); + return Err(Error::invalid_argument( + "split_points must be unique and monotonically increasing", + )); } + Ok(()) } -#[track_caller] -fn check_ranks(ranks: &[f64]) { - assert!( - ranks.iter().all(|rank| (0.0..=1.0).contains(rank)), - "ranks must be in [0.0, 1.0]; got {ranks:?}" - ); +fn check_rank(rank: f64) -> Result<(), Error> { + if !(0.0..=1.0).contains(&rank) { + return Err(Error::invalid_argument(format!( + "rank must be in [0.0, 1.0], got {rank}" + ))); + } + Ok(()) +} + +fn check_ranks(ranks: &[f64]) -> Result<(), Error> { + for &rank in ranks { + check_rank(rank)?; + } + Ok(()) } fn centroid_cmp(a: &Centroid, b: &Centroid) -> Ordering { diff --git a/tests-integration/tests/kll_test/generic.rs b/tests-integration/tests/kll_test/generic.rs index 14d06677..544f6798 100644 --- a/tests-integration/tests/kll_test/generic.rs +++ b/tests-integration/tests/kll_test/generic.rs @@ -68,7 +68,11 @@ fn custom_item_order_controls_queries_and_survives_roundtrip() { assert_eq!(sketch.min_item().map(|item| item.0.as_str()), Some("1")); assert_eq!(sketch.max_item().map(|item| item.0.as_str()), Some("10")); assert_eq!( - sketch.quantile(0.5, SearchCriteria::Inclusive).unwrap().0, + sketch + .quantile(0.5, SearchCriteria::Inclusive) + .unwrap() + .unwrap() + .0, "2" ); @@ -78,7 +82,11 @@ fn custom_item_order_controls_queries_and_survives_roundtrip() { assert_eq!(decoded.min_item().map(|item| item.0.as_str()), Some("1")); assert_eq!(decoded.max_item().map(|item| item.0.as_str()), Some("10")); assert_eq!( - decoded.quantile(0.5, SearchCriteria::Inclusive).unwrap().0, + decoded + .quantile(0.5, SearchCriteria::Inclusive) + .unwrap() + .unwrap() + .0, "2" ); } diff --git a/tests-integration/tests/kll_test/merge.rs b/tests-integration/tests/kll_test/merge.rs index 417fa1b1..513f0dbc 100644 --- a/tests-integration/tests/kll_test/merge.rs +++ b/tests-integration/tests/kll_test/merge.rs @@ -35,6 +35,7 @@ fn merge_preserves_weight_extrema_and_query_invariants() { assert_eq!(left.sorted_view().total_weight(), left.n()); let quantiles = left .quantiles(&[0.0, 0.25, 0.5, 0.75, 1.0], SearchCriteria::Inclusive) + .unwrap() .unwrap(); assert!(quantiles.windows(2).all(|pair| pair[0] <= pair[1])); } diff --git a/tests-integration/tests/kll_test/query.rs b/tests-integration/tests/kll_test/query.rs index 3730224a..aa303e36 100644 --- a/tests-integration/tests/kll_test/query.rs +++ b/tests-integration/tests/kll_test/query.rs @@ -35,18 +35,24 @@ fn quantile_endpoints_preserve_extrema_after_compaction() { let view = sketch.sorted_view(); for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { - assert_eq!(sketch.quantile(0.0, criteria).unwrap(), 0); - assert_eq!(sketch.quantile(1.0, criteria).unwrap(), 7); - assert_eq!(sketch.quantiles(&[0.0, 1.0], criteria).unwrap(), [0, 7]); + assert_eq!(sketch.quantile(0.0, criteria).unwrap(), Some(0)); + assert_eq!(sketch.quantile(1.0, criteria).unwrap(), Some(7)); + assert_eq!( + sketch.quantiles(&[0.0, 1.0], criteria).unwrap(), + Some(vec![0, 7]) + ); } sketch.reset(); for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { - assert!(sketch.quantile(0.0, criteria).is_err()); - assert!(sketch.quantile(1.0, criteria).is_err()); - assert_eq!(view.quantile(0.0, criteria).unwrap(), 0); - assert_eq!(view.quantile(1.0, criteria).unwrap(), 7); - assert_eq!(view.quantiles(&[0.0, 1.0], criteria).unwrap(), [0, 7]); + assert_eq!(sketch.quantile(0.0, criteria).unwrap(), None); + assert_eq!(sketch.quantile(1.0, criteria).unwrap(), None); + assert_eq!(view.quantile(0.0, criteria).unwrap(), Some(0)); + assert_eq!(view.quantile(1.0, criteria).unwrap(), Some(7)); + assert_eq!( + view.quantiles(&[0.0, 1.0], criteria).unwrap(), + Some(vec![0, 7]) + ); } } @@ -62,30 +68,85 @@ fn quantile_endpoints_with_large_stream_weight() { let view = sketch.sorted_view(); for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { - assert_eq!(sketch.quantile(0.0, criteria).unwrap(), 0); - assert_eq!(sketch.quantile(1.0, criteria).unwrap(), 1); - assert_eq!(view.quantile(0.0, criteria).unwrap(), 0); - assert_eq!(view.quantile(1.0, criteria).unwrap(), 1); + assert_eq!(sketch.quantile(0.0, criteria).unwrap(), Some(0)); + assert_eq!(sketch.quantile(1.0, criteria).unwrap(), Some(1)); + assert_eq!(view.quantile(0.0, criteria).unwrap(), Some(0)); + assert_eq!(view.quantile(1.0, criteria).unwrap(), Some(1)); } } #[test] -fn empty_and_invalid_queries_return_errors() { - let mut sketch = KllSketch::::new(DEFAULT_K).unwrap(); - assert!(sketch.rank(&0, SearchCriteria::Inclusive).is_err()); - assert!(sketch.quantile(0.5, SearchCriteria::Inclusive).is_err()); - assert!(sketch.pmf(&[0], SearchCriteria::Inclusive).is_err()); - assert!(sketch.cdf(&[0], SearchCriteria::Inclusive).is_err()); +fn valid_queries_on_empty_sketch_return_no_result() { + let sketch = KllSketch::::new(DEFAULT_K).unwrap(); + let view = sketch.sorted_view(); + for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { + assert_eq!(sketch.rank(&0, criteria), None); + assert_eq!(view.rank(&0, criteria), None); + for rank in [0.0, 0.5, 1.0] { + assert_eq!(sketch.quantile(rank, criteria).unwrap(), None); + assert_eq!(view.quantile(rank, criteria).unwrap(), None); + } + assert_eq!(sketch.quantiles(&[0.0, 0.5, 1.0], criteria).unwrap(), None); + assert_eq!(view.quantiles(&[0.0, 0.5, 1.0], criteria).unwrap(), None); + assert_eq!(sketch.cdf(&[0], criteria).unwrap(), None); + assert_eq!(sketch.pmf(&[0], criteria).unwrap(), None); + assert_eq!(view.cdf(&[0], criteria).unwrap(), None); + assert_eq!(view.pmf(&[0], criteria).unwrap(), None); + } +} - sketch.update(0); - for rank in [-1.0, f64::NAN, 1.1] { - let error = sketch - .quantile(rank, SearchCriteria::Inclusive) - .unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); - } - let error = sketch.cdf(&[1, 0], SearchCriteria::Inclusive).unwrap_err(); - assert_eq!(error.kind(), ErrorKind::InvalidArgument); +#[test] +fn empty_argument_slices_preserve_the_sketch_empty_state() { + for n in [0, 1] { + let mut sketch = KllSketch::::new(DEFAULT_K).unwrap(); + for item in 0..n { + sketch.update(item); + } + let view = sketch.sorted_view(); + let quantiles = (n != 0).then(Vec::new); + let distribution = (n != 0).then(|| vec![1.0]); + for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { + assert_eq!(sketch.quantiles(&[], criteria).unwrap(), quantiles); + assert_eq!(view.quantiles(&[], criteria).unwrap(), quantiles); + assert_eq!(sketch.cdf(&[], criteria).unwrap(), distribution); + assert_eq!(sketch.pmf(&[], criteria).unwrap(), distribution); + assert_eq!(view.cdf(&[], criteria).unwrap(), distribution); + assert_eq!(view.pmf(&[], criteria).unwrap(), distribution); + } + } +} + +#[test] +fn invalid_arguments_are_rejected_regardless_of_sketch_state() { + for n in [0, 1, 1_000] { + let mut sketch = KllSketch::::new(DEFAULT_K).unwrap(); + for item in 0..n { + sketch.update(item); + } + let view = sketch.sorted_view(); + for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { + for rank in [f64::NAN, f64::NEG_INFINITY, -0.1, 1.1, f64::INFINITY] { + for error in [ + sketch.quantile(rank, criteria).unwrap_err(), + view.quantile(rank, criteria).unwrap_err(), + sketch.quantiles(&[0.5, rank], criteria).unwrap_err(), + view.quantiles(&[0.5, rank], criteria).unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } + } + for splits in [[1, 1], [2, 1]] { + for error in [ + sketch.cdf(&splits, criteria).unwrap_err(), + sketch.pmf(&splits, criteria).unwrap_err(), + view.cdf(&splits, criteria).unwrap_err(), + view.pmf(&splits, criteria).unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } + } + } + } } #[test] @@ -99,8 +160,14 @@ fn inclusive_and_exclusive_semantics_cover_duplicates() { assert_eq!(sketch.rank(&1, SearchCriteria::Inclusive).unwrap(), 0.5); assert_eq!(sketch.rank(&2, SearchCriteria::Exclusive).unwrap(), 0.5); assert_eq!(sketch.rank(&2, SearchCriteria::Inclusive).unwrap(), 1.0); - assert_eq!(sketch.quantile(0.5, SearchCriteria::Inclusive).unwrap(), 1); - assert_eq!(sketch.quantile(0.5, SearchCriteria::Exclusive).unwrap(), 2); + assert_eq!( + sketch.quantile(0.5, SearchCriteria::Inclusive).unwrap(), + Some(1) + ); + assert_eq!( + sketch.quantile(0.5, SearchCriteria::Exclusive).unwrap(), + Some(2) + ); } #[test] @@ -110,11 +177,17 @@ fn exact_mode_queries_match_the_stream() { sketch.update(item); } - assert_eq!(sketch.quantile(0.0, SearchCriteria::Inclusive).unwrap(), 1); - assert_eq!(sketch.quantile(0.5, SearchCriteria::Inclusive).unwrap(), 50); + assert_eq!( + sketch.quantile(0.0, SearchCriteria::Inclusive).unwrap(), + Some(1) + ); + assert_eq!( + sketch.quantile(0.5, SearchCriteria::Inclusive).unwrap(), + Some(50) + ); assert_eq!( sketch.quantile(1.0, SearchCriteria::Inclusive).unwrap(), - 100 + Some(100) ); for item in 1..=100 { assert_eq!( @@ -152,8 +225,8 @@ fn rank_cdf_and_pmf_are_consistent() { let split_points: Vec<_> = (100..10_000).step_by(100).collect(); for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { - let cdf = sketch.cdf(&split_points, criteria).unwrap(); - let pmf = sketch.pmf(&split_points, criteria).unwrap(); + let cdf = sketch.cdf(&split_points, criteria).unwrap().unwrap(); + let pmf = sketch.pmf(&split_points, criteria).unwrap().unwrap(); let mut subtotal = 0.0; for (index, split_point) in split_points.iter().enumerate() { subtotal += pmf[index]; @@ -172,18 +245,23 @@ fn sorted_view_supports_repeated_and_batch_queries() { } let view = sketch.sorted_view(); let ranks = [0.0, 0.25, 0.5, 0.75, 1.0]; - let quantiles = sketch.quantiles(&ranks, SearchCriteria::Inclusive).unwrap(); + let quantiles = sketch + .quantiles(&ranks, SearchCriteria::Inclusive) + .unwrap() + .unwrap(); assert_eq!(view.len(), sketch.num_retained()); assert_eq!(view.total_weight(), sketch.n()); assert_eq!( - view.quantiles(&ranks, SearchCriteria::Inclusive).unwrap(), + view.quantiles(&ranks, SearchCriteria::Inclusive) + .unwrap() + .unwrap(), quantiles ); for (&rank, quantile) in ranks.iter().zip(&quantiles) { assert_eq!( view.quantile(rank, SearchCriteria::Inclusive).unwrap(), - *quantile + Some(*quantile) ); assert_eq!( view.rank(quantile, SearchCriteria::Inclusive).unwrap(), @@ -193,5 +271,8 @@ fn sorted_view_supports_repeated_and_batch_queries() { sketch.update(2_000); assert_eq!(view.total_weight(), 1_000); - assert_eq!(view.quantile(1.0, SearchCriteria::Inclusive).unwrap(), 999); + assert_eq!( + view.quantile(1.0, SearchCriteria::Inclusive).unwrap(), + Some(999) + ); } diff --git a/tests-integration/tests/req_test/accuracy.rs b/tests-integration/tests/req_test/accuracy.rs index 76c318cc..0fc361c7 100644 --- a/tests-integration/tests/req_test/accuracy.rs +++ b/tests-integration/tests/req_test/accuracy.rs @@ -38,8 +38,12 @@ fn rank_space_error_is_bounded() -> Result<(), Error> { assert_eq!(sketch.n(), n as u64); for rank in [0.1, 0.25, 0.5, 0.75, 0.9, 0.95, 0.99] { - let quantile = sketch.quantile(rank, SearchCriteria::Inclusive)?; - let estimated_rank = sketch.rank(&quantile, SearchCriteria::Inclusive)?; + let quantile = sketch + .quantile(rank, SearchCriteria::Inclusive)? + .expect("the sketch is non-empty"); + let estimated_rank = sketch + .rank(&quantile, SearchCriteria::Inclusive) + .expect("the sketch is non-empty"); let abs_rank_error = (estimated_rank - rank).abs(); let max_abs_rank_error = if rank >= 0.9 { 0.01 } else { 0.02 }; diff --git a/tests-integration/tests/req_test/bounds.rs b/tests-integration/tests/req_test/bounds.rs index d6ee3a77..a0c61992 100644 --- a/tests-integration/tests/req_test/bounds.rs +++ b/tests-integration/tests/req_test/bounds.rs @@ -77,7 +77,9 @@ fn theoretical_error_bounds_cover_uniform_quantiles() -> Result<(), Error> { 0.95, 0.97, 0.98, 0.99, 0.995, 0.999, ] { let true_quantile = req_f64(rank * (n - 1) as f64); - let estimated_rank = sketch.rank(&true_quantile, SearchCriteria::Inclusive)?; + let estimated_rank = sketch + .rank(&true_quantile, SearchCriteria::Inclusive) + .expect("the sketch is non-empty"); let lower = sketch.rank_lower_bound(rank, NumStdDev::Three); let upper = sketch.rank_upper_bound(rank, NumStdDev::Three); assert_that!(estimated_rank, all!(ge(lower), le(upper)), "rank: {rank}"); diff --git a/tests-integration/tests/req_test/core.rs b/tests-integration/tests/req_test/core.rs index 626632db..1a3622e7 100644 --- a/tests-integration/tests/req_test/core.rs +++ b/tests-integration/tests/req_test/core.rs @@ -42,7 +42,7 @@ use super::req_f32; use super::req_f64; #[test] -fn empty_sketch_has_default_state_and_rejects_queries() { +fn empty_sketch_has_default_state_and_absent_queries() { let sketch: ReqSketch = ReqSketch::default(); assert_eq!(sketch.k(), 12); @@ -53,22 +53,23 @@ fn empty_sketch_has_default_state_and_rejects_queries() { assert_that!(sketch.min_item(), none()); assert_that!(sketch.max_item(), none()); - assert_that!( - sketch.rank(&req_f64(0.0), SearchCriteria::Inclusive), - err(anything()) - ); - assert_that!( + assert_eq!(sketch.rank(&req_f64(0.0), SearchCriteria::Inclusive), None); + assert!(matches!( sketch.quantile(0.5, SearchCriteria::Inclusive), - err(anything()) - ); - assert_that!( + Ok(None) + )); + assert!(matches!( + sketch.quantiles(&[0.25, 0.75], SearchCriteria::Inclusive), + Ok(None) + )); + assert!(matches!( sketch.pmf(&[req_f64(0.0)], SearchCriteria::Inclusive), - err(anything()) - ); - assert_that!( + Ok(None) + )); + assert!(matches!( sketch.cdf(&[req_f64(0.0)], SearchCriteria::Inclusive), - err(anything()) - ); + Ok(None) + )); } #[test] @@ -112,7 +113,8 @@ fn single_value_hra_answers_exactly() { assert_eq!( sketch .quantile(rank, SearchCriteria::Exclusive) - .expect("quantile should succeed"), + .expect("quantile should succeed") + .expect("the sketch is non-empty"), req_f32(1.0) ); } @@ -221,14 +223,18 @@ fn small_edge_cases_answer_reasonably() -> Result<(), Error> { let mut single: ReqSketch = ReqSketch::default(); single.update(req_f64(42.0)); assert_eq!( - single.quantile(0.5, SearchCriteria::Inclusive)?, + single + .quantile(0.5, SearchCriteria::Inclusive)? + .expect("the sketch is non-empty"), req_f64(42.0) ); let mut two_values = ReqSketch::default(); two_values.update(req_f64(1.0)); two_values.update(req_f64(100.0)); - let median = two_values.quantile(0.5, SearchCriteria::Inclusive)?; + let median = two_values + .quantile(0.5, SearchCriteria::Inclusive)? + .expect("the sketch is non-empty"); assert_that!(*median, all!(ge(1.0), le(100.0))); let mut duplicates = ReqSketch::default(); @@ -236,7 +242,9 @@ fn small_edge_cases_answer_reasonably() -> Result<(), Error> { duplicates.update(req_f64(42.0)); } assert_eq!( - duplicates.quantile(0.5, SearchCriteria::Inclusive)?, + duplicates + .quantile(0.5, SearchCriteria::Inclusive)? + .expect("the sketch is non-empty"), req_f64(42.0) ); diff --git a/tests-integration/tests/req_test/generic.rs b/tests-integration/tests/req_test/generic.rs index a405e815..ab4335b5 100644 --- a/tests-integration/tests/req_test/generic.rs +++ b/tests-integration/tests/req_test/generic.rs @@ -31,7 +31,10 @@ fn custom_items_do_not_need_serialization() { assert_eq!(sketch.min_item(), Some(&Reading(10))); assert_eq!(sketch.max_item(), Some(&Reading(30))); assert_eq!( - sketch.quantile(0.5, SearchCriteria::Inclusive).unwrap(), + sketch + .quantile(0.5, SearchCriteria::Inclusive) + .unwrap() + .unwrap(), Reading(20) ); } diff --git a/tests-integration/tests/req_test/merge.rs b/tests-integration/tests/req_test/merge.rs index d4c80e29..d24d6e7c 100644 --- a/tests-integration/tests/req_test/merge.rs +++ b/tests-integration/tests/req_test/merge.rs @@ -43,13 +43,16 @@ fn merge_into_empty_preserves_source_distribution() { let q25 = target .quantile(0.25, SearchCriteria::Inclusive) - .expect("quantile should succeed"); + .expect("quantile should succeed") + .expect("the sketch is non-empty"); let q50 = target .quantile(0.5, SearchCriteria::Inclusive) - .expect("quantile should succeed"); + .expect("quantile should succeed") + .expect("the sketch is non-empty"); let q75 = target .quantile(0.75, SearchCriteria::Inclusive) - .expect("quantile should succeed"); + .expect("quantile should succeed") + .expect("the sketch is non-empty"); let r50 = target .rank(&req_f64(500.0), SearchCriteria::Inclusive) .expect("rank should succeed"); @@ -78,13 +81,16 @@ fn merge_two_ranges_preserves_distribution() { let q25 = left .quantile(0.25, SearchCriteria::Inclusive) - .expect("quantile should succeed"); + .expect("quantile should succeed") + .expect("the sketch is non-empty"); let q50 = left .quantile(0.5, SearchCriteria::Inclusive) - .expect("quantile should succeed"); + .expect("quantile should succeed") + .expect("the sketch is non-empty"); let q75 = left .quantile(0.75, SearchCriteria::Inclusive) - .expect("quantile should succeed"); + .expect("quantile should succeed") + .expect("the sketch is non-empty"); let r50 = left .rank(&req_f64(1000.0), SearchCriteria::Inclusive) .expect("rank should succeed"); @@ -122,6 +128,7 @@ fn many_small_merges_preserve_count_bounds_and_median() { let median = sketch .quantile(0.5, SearchCriteria::Inclusive) - .expect("quantile should succeed"); + .expect("quantile should succeed") + .expect("the sketch is non-empty"); assert_that!(*median, near(4999.5, 500.0)); } diff --git a/tests-integration/tests/req_test/property.rs b/tests-integration/tests/req_test/property.rs index cf7a8bd5..24d91df7 100644 --- a/tests-integration/tests/req_test/property.rs +++ b/tests-integration/tests/req_test/property.rs @@ -48,7 +48,8 @@ fn prop_quantile_rank_consistency() { for rank in [0.1, 0.25, 0.5, 0.75, 0.9] { let quantile = sketch .quantile(rank, SearchCriteria::Inclusive) - .expect("quantile should succeed"); + .expect("quantile should succeed") + .expect("the sketch is non-empty"); let recovered = sketch .rank(&quantile, SearchCriteria::Inclusive) .expect("rank should succeed"); @@ -96,7 +97,8 @@ fn prop_sketch_bounds() { for rank in [0.0, 0.25, 0.5, 0.75, 1.0] { let quantile = sketch .quantile(rank, SearchCriteria::Inclusive) - .expect("quantile should succeed"); + .expect("quantile should succeed") + .expect("the sketch is non-empty"); assert!( quantile >= true_min && quantile <= true_max, "quantile {} out of bounds [{}, {}]", diff --git a/tests-integration/tests/req_test/query.rs b/tests-integration/tests/req_test/query.rs index bc037434..20d2fda6 100644 --- a/tests-integration/tests/req_test/query.rs +++ b/tests-integration/tests/req_test/query.rs @@ -64,7 +64,8 @@ fn exact_mode_rank_quantile_pmf_and_cdf_match_reference() { assert_eq!( *sketch .quantile(rank, SearchCriteria::Exclusive) - .expect("quantile should succeed"), + .expect("quantile should succeed") + .expect("the sketch is non-empty"), expected ); } @@ -73,7 +74,8 @@ fn exact_mode_rank_quantile_pmf_and_cdf_match_reference() { assert_eq!( *sketch .quantile(rank, SearchCriteria::Inclusive) - .expect("quantile should succeed"), + .expect("quantile should succeed") + .expect("the sketch is non-empty"), expected ); } @@ -81,7 +83,8 @@ fn exact_mode_rank_quantile_pmf_and_cdf_match_reference() { let splits = [2.0, 6.0, 9.0].map(req_f64); let cdf = sketch .cdf(&splits, SearchCriteria::Exclusive) - .expect("cdf should succeed"); + .expect("cdf should succeed") + .expect("the sketch is non-empty"); assert_that!(cdf[0], near(0.1, 1e-6)); assert_that!(cdf[1], near(0.5, 1e-6)); assert_that!(cdf[2], near(0.8, 1e-6)); @@ -89,7 +92,8 @@ fn exact_mode_rank_quantile_pmf_and_cdf_match_reference() { let pmf = sketch .pmf(&splits, SearchCriteria::Exclusive) - .expect("pmf should succeed"); + .expect("pmf should succeed") + .expect("the sketch is non-empty"); assert_that!(pmf[0], near(0.1, 1e-6)); assert_that!(pmf[1], near(0.4, 1e-6)); assert_that!(pmf[2], near(0.3, 1e-6)); @@ -106,10 +110,12 @@ fn pmf_and_cdf_are_consistent() { let split_points = [100.0, 300.0, 500.0, 700.0, 900.0].map(req_f64); let pmf = sketch .pmf(&split_points, SearchCriteria::Inclusive) - .expect("pmf should succeed"); + .expect("pmf should succeed") + .expect("the sketch is non-empty"); let cdf = sketch .cdf(&split_points, SearchCriteria::Inclusive) - .expect("cdf should succeed"); + .expect("cdf should succeed") + .expect("the sketch is non-empty"); assert_that!(pmf.iter().sum::(), near(1.0, 1e-10)); @@ -139,9 +145,9 @@ fn cdf_preserves_ranks_and_exact_endpoint() { .chain(std::iter::once(1.0)) .collect(); - assert_eq!(sketch.cdf(&splits, criteria).unwrap(), expected); - assert_eq!(view.cdf(&splits, criteria).unwrap(), expected); - assert_eq!(view.cdf(&[], criteria).unwrap(), [1.0]); + assert_eq!(sketch.cdf(&splits, criteria).unwrap().unwrap(), expected); + assert_eq!(view.cdf(&splits, criteria).unwrap().unwrap(), expected); + assert_eq!(view.cdf(&[], criteria).unwrap(), Some(vec![1.0])); } } @@ -157,14 +163,14 @@ fn quantile_endpoints_preserve_extrema_after_compaction() { let view = sketch.sorted_view(); for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { - assert_eq!(sketch.quantile(0.0, criteria).unwrap(), -item); - assert_eq!(sketch.quantile(1.0, criteria).unwrap(), item); + assert_eq!(sketch.quantile(0.0, criteria).unwrap(), Some(-item)); + assert_eq!(sketch.quantile(1.0, criteria).unwrap(), Some(item)); assert_eq!( sketch.quantiles(&[0.0, 1.0], criteria).unwrap(), - [-item, item] + Some(vec![-item, item]) ); - assert_eq!(view.quantile(0.0, criteria).unwrap(), -item); - assert_eq!(view.quantile(1.0, criteria).unwrap(), item); + assert_eq!(view.quantile(0.0, criteria).unwrap(), Some(-item)); + assert_eq!(view.quantile(1.0, criteria).unwrap(), Some(item)); } } assert!(sketch.is_estimation_mode()); @@ -172,10 +178,10 @@ fn quantile_endpoints_preserve_extrema_after_compaction() { let view = sketch.sorted_view(); sketch.reset(); for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { - assert!(sketch.quantile(0.0, criteria).is_err()); - assert!(sketch.quantile(1.0, criteria).is_err()); - assert_eq!(view.quantile(0.0, criteria).unwrap(), -64); - assert_eq!(view.quantile(1.0, criteria).unwrap(), 64); + assert_eq!(sketch.quantile(0.0, criteria).unwrap(), None); + assert_eq!(sketch.quantile(1.0, criteria).unwrap(), None); + assert_eq!(view.quantile(0.0, criteria).unwrap(), Some(-64)); + assert_eq!(view.quantile(1.0, criteria).unwrap(), Some(64)); } } } @@ -192,10 +198,10 @@ fn quantile_endpoints_with_large_stream_weight() { let view = sketch.sorted_view(); for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { - assert_eq!(sketch.quantile(0.0, criteria).unwrap(), 0); - assert_eq!(sketch.quantile(1.0, criteria).unwrap(), 1); - assert_eq!(view.quantile(0.0, criteria).unwrap(), 0); - assert_eq!(view.quantile(1.0, criteria).unwrap(), 1); + assert_eq!(sketch.quantile(0.0, criteria).unwrap(), Some(0)); + assert_eq!(sketch.quantile(1.0, criteria).unwrap(), Some(1)); + assert_eq!(view.quantile(0.0, criteria).unwrap(), Some(0)); + assert_eq!(view.quantile(1.0, criteria).unwrap(), Some(1)); } } @@ -230,7 +236,9 @@ fn quantiles_are_monotonic() -> Result<(), Error> { let mut previous = 0.0; for rank in ranks { - let quantile = sketch.quantile(rank, SearchCriteria::Inclusive)?; + let quantile = sketch + .quantile(rank, SearchCriteria::Inclusive)? + .expect("the sketch is non-empty"); assert_that!(*quantile, ge(previous)); previous = *quantile; } @@ -246,8 +254,12 @@ fn rank_quantile_round_trip_is_consistent() -> Result<(), Error> { } for target_rank in [0.1, 0.25, 0.5, 0.75, 0.9] { - let quantile = sketch.quantile(target_rank, SearchCriteria::Inclusive)?; - let recovered_rank = sketch.rank(&quantile, SearchCriteria::Inclusive)?; + let quantile = sketch + .quantile(target_rank, SearchCriteria::Inclusive)? + .expect("the sketch is non-empty"); + let recovered_rank = sketch + .rank(&quantile, SearchCriteria::Inclusive) + .expect("the sketch is non-empty"); let error = (recovered_rank - target_rank).abs() / target_rank; assert_that!(error, lt(0.2)); } @@ -263,8 +275,12 @@ fn search_criteria_rank_consistency() -> Result<(), Error> { } for value in [100.0, 250.0, 500.0, 750.0].map(req_f64) { - let inclusive_rank = sketch.rank(&value, SearchCriteria::Inclusive)?; - let exclusive_rank = sketch.rank(&value, SearchCriteria::Exclusive)?; + let inclusive_rank = sketch + .rank(&value, SearchCriteria::Inclusive) + .expect("the sketch is non-empty"); + let exclusive_rank = sketch + .rank(&value, SearchCriteria::Exclusive) + .expect("the sketch is non-empty"); assert_that!(exclusive_rank, le(inclusive_rank)); assert_that!(inclusive_rank, all!(ge(0.0), le(1.0))); @@ -283,8 +299,18 @@ fn signed_zeros_share_rank_and_cannot_be_distinct_splits() -> Result<(), Error> sketch.update(positive_zero); for value in [negative_zero, positive_zero] { - assert_eq!(sketch.rank(&value, SearchCriteria::Exclusive)?, 0.0); - assert_eq!(sketch.rank(&value, SearchCriteria::Inclusive)?, 1.0); + assert_eq!( + sketch + .rank(&value, SearchCriteria::Exclusive) + .expect("the sketch is non-empty"), + 0.0 + ); + assert_eq!( + sketch + .rank(&value, SearchCriteria::Inclusive) + .expect("the sketch is non-empty"), + 1.0 + ); } assert!( diff --git a/tests-integration/tests/req_test/sorted_view_api.rs b/tests-integration/tests/req_test/sorted_view_api.rs index 59c4e6dc..4c4d3d71 100644 --- a/tests-integration/tests/req_test/sorted_view_api.rs +++ b/tests-integration/tests/req_test/sorted_view_api.rs @@ -25,9 +25,6 @@ use datasketches::req::ReqSketch; use datasketches::req::SortedView; use googletest::assert_that; use googletest::prelude::all; -use googletest::prelude::anything; -use googletest::prelude::contains_substring; -use googletest::prelude::err; use googletest::prelude::ge; use googletest::prelude::lt; use googletest::prelude::near; @@ -47,19 +44,23 @@ fn populated_sketch(n: i64) -> ReqSketch { fn query_through_shared_ref(sketch: &ReqSketch) { sketch .quantile(0.5, SearchCriteria::Inclusive) - .expect("quantile"); + .expect("quantile") + .expect("the sketch is non-empty"); sketch .quantiles(&[0.25, 0.5, 0.75], SearchCriteria::Inclusive) - .expect("quantiles"); + .expect("quantiles") + .expect("the sketch is non-empty"); sketch .rank(&req_f64(50.0), SearchCriteria::Inclusive) .expect("rank"); sketch .pmf(&[req_f64(10.0), req_f64(50.0)], SearchCriteria::Inclusive) - .expect("pmf"); + .expect("pmf") + .expect("the sketch is non-empty"); sketch .cdf(&[req_f64(10.0), req_f64(50.0)], SearchCriteria::Inclusive) - .expect("cdf"); + .expect("cdf") + .expect("the sketch is non-empty"); assert!(!sketch.sorted_view().is_empty()); } @@ -95,24 +96,36 @@ fn sorted_view_on_empty_sketch_is_an_empty_view() { assert!(view.is_empty()); assert_eq!(view.len(), 0); assert_eq!(view.total_weight(), 0); - // Queries on the empty view still report an error. - assert_that!( - view.quantile(0.5, SearchCriteria::Inclusive), - err(anything()) - ); + for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { + assert_eq!(sketch.rank(&req_f64(0.0), criteria), None); + assert_eq!(view.rank(&req_f64(0.0), criteria), None); + for rank in [0.0, 0.5, 1.0] { + assert_eq!(sketch.quantile(rank, criteria).unwrap(), None); + assert_eq!(view.quantile(rank, criteria).unwrap(), None); + } + assert_eq!(sketch.quantiles(&[0.0, 0.5, 1.0], criteria).unwrap(), None); + assert_eq!(sketch.pmf(&[req_f64(0.0)], criteria).unwrap(), None); + assert_eq!(sketch.cdf(&[req_f64(0.0)], criteria).unwrap(), None); + assert_eq!(view.pmf(&[req_f64(0.0)], criteria).unwrap(), None); + assert_eq!(view.cdf(&[req_f64(0.0)], criteria).unwrap(), None); + } } #[test] -fn empty_sketch_pmf_cdf_report_error() { - let sketch: ReqSketch = ReqSketch::default(); - assert_that!( - sketch.pmf(&[req_f64(1.0)], SearchCriteria::Inclusive), - err(anything()) - ); - assert_that!( - sketch.cdf(&[req_f64(1.0)], SearchCriteria::Inclusive), - err(anything()) - ); +fn empty_argument_slices_preserve_the_sketch_empty_state() { + for n in [0, 1] { + let sketch = populated_sketch(n); + let view = sketch.sorted_view(); + let quantiles = (n != 0).then(Vec::new); + let distribution = (n != 0).then(|| vec![1.0]); + for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { + assert_eq!(sketch.quantiles(&[], criteria).unwrap(), quantiles); + assert_eq!(sketch.cdf(&[], criteria).unwrap(), distribution); + assert_eq!(sketch.pmf(&[], criteria).unwrap(), distribution); + assert_eq!(view.cdf(&[], criteria).unwrap(), distribution); + assert_eq!(view.pmf(&[], criteria).unwrap(), distribution); + } + } } #[test] @@ -126,17 +139,33 @@ fn view_rank_is_primary_query_name() { } #[test] -fn error_precedence_empty_before_invalid_rank() { - // On an empty sketch the emptiness is reported before the out-of-range rank. - let empty: ReqSketch = ReqSketch::default(); - let empty_err = empty.quantile(2.0, SearchCriteria::Inclusive).unwrap_err(); - assert_that!(empty_err.message(), contains_substring("empty")); - - // On a populated sketch the out-of-range rank is reported. - let sketch = populated_sketch(10); - let range_err = sketch.quantile(2.0, SearchCriteria::Inclusive).unwrap_err(); - assert_eq!(range_err.kind(), ErrorKind::InvalidArgument); - assert_that!(range_err.message(), contains_substring("must be in")); +fn invalid_arguments_are_rejected_regardless_of_sketch_state() { + for n in [0, 1, 1_000] { + let sketch = populated_sketch(n); + let view = sketch.sorted_view(); + for criteria in [SearchCriteria::Inclusive, SearchCriteria::Exclusive] { + for rank in [f64::NAN, f64::NEG_INFINITY, -0.1, 1.1, f64::INFINITY] { + for error in [ + sketch.quantile(rank, criteria).unwrap_err(), + view.quantile(rank, criteria).unwrap_err(), + sketch.quantiles(&[0.5, rank], criteria).unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } + } + for splits in [[1.0, 1.0], [2.0, 1.0]] { + let splits = splits.map(req_f64); + for error in [ + sketch.cdf(&splits, criteria).unwrap_err(), + sketch.pmf(&splits, criteria).unwrap_err(), + view.cdf(&splits, criteria).unwrap_err(), + view.pmf(&splits, criteria).unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } + } + } + } } #[test] @@ -157,6 +186,7 @@ fn concurrent_readers_share_the_sketch() { sketch .quantile(rank, SearchCriteria::Inclusive) .expect("quantile from shared sketch") + .expect("the sketch is non-empty") }) }) .collect(); diff --git a/tests-integration/tests/serde_tests/req.rs b/tests-integration/tests/serde_tests/req.rs index d0a03e1e..a3a920f6 100644 --- a/tests-integration/tests/serde_tests/req.rs +++ b/tests-integration/tests/serde_tests/req.rs @@ -510,7 +510,10 @@ fn validate_cross_language_fixture(path: PathBuf, expected_n: u64) { sketch.max_item().copied().map(ReqFloat::into_inner), Some(expected_n as f32) ); - let _ = sketch.quantile(0.5, SearchCriteria::Inclusive).unwrap(); + sketch + .quantile(0.5, SearchCriteria::Inclusive) + .unwrap() + .expect("non-empty snapshot should have a median"); } let serialized = sketch.serialize(); diff --git a/tests-integration/tests/serde_tests/tdigest.rs b/tests-integration/tests/serde_tests/tdigest.rs index b90b1ca6..8bcd9890 100644 --- a/tests-integration/tests/serde_tests/tdigest.rs +++ b/tests-integration/tests/serde_tests/tdigest.rs @@ -64,13 +64,17 @@ fn test_sketch_file(path: PathBuf, n: u64, with_buffer: bool, is_f32: bool) { assert_eq!(td.total_weight(), n, "filepath: {path}"); assert_eq!(td.min_value(), Some(1.0), "filepath: {path}"); assert_eq!(td.max_value(), Some(n as f64), "filepath: {path}"); - assert_eq!(td.rank(0.0), Some(0.0), "filepath: {path}"); - assert_eq!(td.rank((n + 1) as f64), Some(1.0), "filepath: {path}"); + assert_eq!(td.rank(0.0).unwrap(), Some(0.0), "filepath: {path}"); + assert_eq!( + td.rank((n + 1) as f64).unwrap(), + Some(1.0), + "filepath: {path}" + ); if n == 1 { - assert_eq!(td.rank(n as f64), Some(0.5), "filepath: {path}"); + assert_eq!(td.rank(n as f64).unwrap(), Some(0.5), "filepath: {path}"); } else { assert_that!( - td.rank(n as f64 / 2.).unwrap(), + td.rank(n as f64 / 2.).unwrap().unwrap(), near(0.5, 0.05), "filepath: {path}", ); @@ -126,23 +130,31 @@ fn test_deserialize_from_reference_implementation() { assert_eq!(td.total_weight(), n, "filepath: {path}"); assert_eq!(td.min_value(), Some(0.0), "filepath: {path}"); assert_eq!(td.max_value(), Some((n - 1) as f64), "filepath: {path}"); - assert_that!(td.rank(0.0).unwrap(), near(0.0, 0.0001), "filepath: {path}"); assert_that!( - td.rank(n as f64 / 4.).unwrap(), + td.rank(0.0).unwrap().unwrap(), + near(0.0, 0.0001), + "filepath: {path}" + ); + assert_that!( + td.rank(n as f64 / 4.).unwrap().unwrap(), near(0.25, 0.0001), "filepath: {path}" ); assert_that!( - td.rank(n as f64 / 2.).unwrap(), + td.rank(n as f64 / 2.).unwrap().unwrap(), near(0.5, 0.0001), "filepath: {path}" ); assert_that!( - td.rank((n * 3) as f64 / 4.).unwrap(), + td.rank((n * 3) as f64 / 4.).unwrap().unwrap(), near(0.75, 0.0001), "filepath: {path}" ); - assert_that!(td.rank(n as f64).unwrap(), eq(1.0), "filepath: {path}"); + assert_that!( + td.rank(n as f64).unwrap().unwrap(), + eq(1.0), + "filepath: {path}" + ); } } @@ -223,8 +235,14 @@ fn test_many_values() { assert_eq!(td.is_empty(), deserialized_td.is_empty()); assert_eq!(td.min_value(), deserialized_td.min_value()); assert_eq!(td.max_value(), deserialized_td.max_value()); - assert_eq!(td.rank(500.0), deserialized_td.rank(500.0)); - assert_eq!(td.quantile(0.5), deserialized_td.quantile(0.5)); + assert_eq!( + td.rank(500.0).unwrap(), + deserialized_td.rank(500.0).unwrap() + ); + assert_eq!( + td.quantile(0.5).unwrap(), + deserialized_td.quantile(0.5).unwrap() + ); } #[test] @@ -239,7 +257,10 @@ fn test_frozen_roundtrip() { assert_eq!(actual.total_weight(), expected.total_weight()); assert_eq!(actual.min_value(), expected.min_value()); assert_eq!(actual.max_value(), expected.max_value()); - assert_eq!(actual.quantile(0.5), expected.quantile(0.5)); + assert_eq!( + actual.quantile(0.5).unwrap(), + expected.quantile(0.5).unwrap() + ); } #[test] @@ -251,7 +272,7 @@ fn test_serialized_bytes_stable_for_full_and_merged_digests() { let mut left = patterned_digest(10, 201, 2); let mut right = patterned_digest(10, 199, 3); - right.rank(0.0); + right.rank(0.0).unwrap(); left.merge(&right); let bytes = left.serialize(); assert_eq!(bytes.len(), 272); @@ -394,7 +415,10 @@ fn test_deserialize_single_sample_requires_equal_extrema() { if min == max { let mut digest = result.unwrap(); assert_eq!(digest.total_weight(), 1); - assert_eq!(digest.quantiles(&[0., 0.5, 1.]), Some(vec![50.; 3])); + assert_eq!( + digest.quantiles(&[0., 0.5, 1.]).unwrap(), + Some(vec![50.; 3]) + ); } else { assert_eq!( result.unwrap_err().kind(), @@ -428,7 +452,10 @@ fn test_deserialize_compat_single_sample_requires_equal_extrema() { if min == max { let mut digest = TDigestMut::deserialize(&bytes).unwrap(); assert_eq!(digest.total_weight(), 1); - assert_eq!(digest.quantiles(&[0., 0.5, 1.]), Some(vec![50.; 3])); + assert_eq!( + digest.quantiles(&[0., 0.5, 1.]).unwrap(), + Some(vec![50.; 3]) + ); } else { assert_invalid_tdigest(&bytes); } @@ -504,6 +531,6 @@ fn test_large_weights_produce_finite_extreme_quantile() { bytes[56..64].copy_from_slice(&(1_u64 << 52).to_le_bytes()); let mut tdigest = TDigestMut::deserialize(&bytes).unwrap(); - let quantile = tdigest.quantile(0.25).unwrap(); + let quantile = tdigest.quantile(0.25).unwrap().unwrap(); assert_that!(quantile, all!(is_finite(), ge(lower), le(f64::MAX))); } diff --git a/tests-integration/tests/tdigest_test/property.rs b/tests-integration/tests/tdigest_test/property.rs index 61577efd..431da0ef 100644 --- a/tests-integration/tests/tdigest_test/property.rs +++ b/tests-integration/tests/tdigest_test/property.rs @@ -39,10 +39,10 @@ fn assert_quantiles_are_monotonic(tdigest: &mut TDigestMut) { let ranks = (0..=RANK_STEPS) .map(|step| step as f64 / RANK_STEPS as f64) .collect::>(); - let quantiles = tdigest.quantiles(&ranks).unwrap(); + let quantiles = tdigest.quantiles(&ranks).unwrap().unwrap(); for (&rank, quantile) in ranks.iter().zip(quantiles) { - assert_eq!(tdigest.quantile(rank), Some(quantile)); + assert_eq!(tdigest.quantile(rank).unwrap(), Some(quantile)); assert!( (previous..=max).contains(&quantile), "quantile {quantile} at rank {rank} is outside [{previous}, {max}]" @@ -77,12 +77,12 @@ fn prop_finite_values_survive_partial_aggregation() { // Use the input samples as split points, independently of the computed quantiles. values.sort_unstable_by(f64::total_cmp); values.dedup(); - assert_eq!(digest.quantile(0.), values.first().copied()); - assert_eq!(digest.quantile(1.), values.last().copied()); - let cdf = digest.cdf(&values).unwrap(); + assert_eq!(digest.quantile(0.).unwrap(), values.first().copied()); + assert_eq!(digest.quantile(1.).unwrap(), values.last().copied()); + let cdf = digest.cdf(&values).unwrap().unwrap(); assert!(cdf.is_sorted()); assert!(cdf.iter().all(|rank| (0.0..=1.0).contains(rank))); - let pmf = digest.pmf(&values).unwrap(); + let pmf = digest.pmf(&values).unwrap().unwrap(); assert!(pmf.iter().all(|p| (0.0..=1.0).contains(p))); assert!((pmf.iter().sum::() - 1.).abs() < 1e-12); TestResult::passed() @@ -159,12 +159,12 @@ fn prop_batch_quantiles_preserve_input_order_and_duplicates() { Some( ranks .iter() - .map(|&rank| frozen.quantile(rank).unwrap()) + .map(|&rank| frozen.quantile(rank).unwrap().unwrap()) .collect::>(), ) }; - assert_eq!(tdigest.clone().quantiles(&ranks), expected); - assert_eq!(frozen.quantiles(&ranks), expected); + assert_eq!(tdigest.clone().quantiles(&ranks).unwrap(), expected); + assert_eq!(frozen.quantiles(&ranks).unwrap(), expected); } } diff --git a/tests-integration/tests/tdigest_test/sketch.rs b/tests-integration/tests/tdigest_test/sketch.rs index b5bb67e4..86bc45fb 100644 --- a/tests-integration/tests/tdigest_test/sketch.rs +++ b/tests-integration/tests/tdigest_test/sketch.rs @@ -19,6 +19,7 @@ use std::mem::size_of; use std::panic::AssertUnwindSafe; use std::panic::catch_unwind; +use datasketches::error::ErrorKind; use datasketches::tdigest::TDigestMut; use googletest::assert_that; use googletest::prelude::eq; @@ -33,14 +34,16 @@ fn test_empty() { assert_eq!(tdigest.total_weight(), 0); assert_eq!(tdigest.min_value(), None); assert_eq!(tdigest.max_value(), None); - assert_eq!(tdigest.rank(0.0), None); - assert_eq!(tdigest.quantile(0.5), None); - assert_eq!(tdigest.quantiles(&[0.0, 0.5, 1.0]), None); - assert_eq!(tdigest.quantiles(&[]), None); + assert_eq!(tdigest.rank(0.0).unwrap(), None); + assert_eq!(tdigest.quantile(0.5).unwrap(), None); + assert_eq!(tdigest.quantiles(&[0.0, 0.5, 1.0]).unwrap(), None); + assert_eq!(tdigest.quantiles(&[]).unwrap(), None); let split_points = [0.0]; - assert_eq!(tdigest.pmf(&split_points), None); - assert_eq!(tdigest.cdf(&split_points), None); + assert_eq!(tdigest.pmf(&split_points).unwrap(), None); + assert_eq!(tdigest.cdf(&split_points).unwrap(), None); + assert_eq!(tdigest.pmf(&[]).unwrap(), None); + assert_eq!(tdigest.cdf(&[]).unwrap(), None); let tdigest = TDigestMut::new(10).unwrap().freeze(); assert!(tdigest.is_empty()); @@ -48,14 +51,16 @@ fn test_empty() { assert_eq!(tdigest.total_weight(), 0); assert_eq!(tdigest.min_value(), None); assert_eq!(tdigest.max_value(), None); - assert_eq!(tdigest.rank(0.0), None); - assert_eq!(tdigest.quantile(0.5), None); - assert_eq!(tdigest.quantiles(&[0.0, 0.5, 1.0]), None); - assert_eq!(tdigest.quantiles(&[]), None); + assert_eq!(tdigest.rank(0.0).unwrap(), None); + assert_eq!(tdigest.quantile(0.5).unwrap(), None); + assert_eq!(tdigest.quantiles(&[0.0, 0.5, 1.0]).unwrap(), None); + assert_eq!(tdigest.quantiles(&[]).unwrap(), None); let split_points = [0.0]; - assert_eq!(tdigest.pmf(&split_points), None); - assert_eq!(tdigest.cdf(&split_points), None); + assert_eq!(tdigest.pmf(&split_points).unwrap(), None); + assert_eq!(tdigest.cdf(&split_points).unwrap(), None); + assert_eq!(tdigest.pmf(&[]).unwrap(), None); + assert_eq!(tdigest.cdf(&[]).unwrap(), None); } #[test] @@ -66,31 +71,36 @@ fn test_one_value() { assert_eq!(tdigest.total_weight(), 1); assert_eq!(tdigest.min_value(), Some(1.0)); assert_eq!(tdigest.max_value(), Some(1.0)); - assert_eq!(tdigest.rank(0.99), Some(0.0)); - assert_eq!(tdigest.rank(1.0), Some(0.5)); - assert_eq!(tdigest.rank(1.01), Some(1.0)); - assert_eq!(tdigest.quantile(0.0), Some(1.0)); - assert_eq!(tdigest.quantile(0.5), Some(1.0)); - assert_eq!(tdigest.quantile(1.0), Some(1.0)); + assert_eq!(tdigest.rank(0.99).unwrap(), Some(0.0)); + assert_eq!(tdigest.rank(1.0).unwrap(), Some(0.5)); + assert_eq!(tdigest.rank(1.01).unwrap(), Some(1.0)); + assert_eq!(tdigest.quantile(0.0).unwrap(), Some(1.0)); + assert_eq!(tdigest.quantile(0.5).unwrap(), Some(1.0)); + assert_eq!(tdigest.quantile(1.0).unwrap(), Some(1.0)); let ranks = [1.0, 0.5, 0.0, 0.5]; - assert_eq!(tdigest.quantiles(&ranks), Some(vec![1.0; ranks.len()])); assert_eq!( - tdigest.freeze().quantiles(&ranks), + tdigest.quantiles(&ranks).unwrap(), + Some(vec![1.0; ranks.len()]) + ); + assert_eq!( + tdigest.freeze().quantiles(&ranks).unwrap(), Some(vec![1.0; ranks.len()]) ); } #[test] -fn test_empty_split_points_define_one_bin() { +fn test_empty_query_slices_on_nonempty_digest() { let mut tdigest = TDigestMut::new(100).unwrap(); tdigest.update(1.0); - assert_eq!(tdigest.cdf(&[]), Some(vec![1.0])); - assert_eq!(tdigest.pmf(&[]), Some(vec![1.0])); + assert_eq!(tdigest.cdf(&[]).unwrap(), Some(vec![1.0])); + assert_eq!(tdigest.pmf(&[]).unwrap(), Some(vec![1.0])); + assert_eq!(tdigest.quantiles(&[]).unwrap(), Some(vec![])); let tdigest = tdigest.freeze(); - assert_eq!(tdigest.cdf(&[]), Some(vec![1.0])); - assert_eq!(tdigest.pmf(&[]), Some(vec![1.0])); + assert_eq!(tdigest.cdf(&[]).unwrap(), Some(vec![1.0])); + assert_eq!(tdigest.pmf(&[]).unwrap(), Some(vec![1.0])); + assert_eq!(tdigest.quantiles(&[]).unwrap(), Some(vec![])); } #[test] @@ -100,7 +110,7 @@ fn test_maximum_k() { let tdigest = tdigest.freeze(); assert_eq!(tdigest.k(), u16::MAX); - assert_eq!(tdigest.quantile(0.5), Some(1.0)); + assert_eq!(tdigest.quantile(0.5).unwrap(), Some(1.0)); } #[test] @@ -118,14 +128,14 @@ fn test_estimated_size_reuses_buffer_after_compression() { } let size_before_compression = tdigest.estimated_size(); assert!(size_before_compression > inline_size); - tdigest.rank(0.5); + tdigest.rank(0.5).unwrap(); assert!(tdigest.estimated_size() <= size_before_compression); for value in MAX_UNMERGED..10_000 { tdigest.update(value as f64); } let size_before_compression = tdigest.estimated_size(); - tdigest.rank(0.5); + tdigest.rank(0.5).unwrap(); assert!(tdigest.estimated_size() <= size_before_compression); let mut left = TDigestMut::new(K).unwrap(); @@ -170,35 +180,41 @@ fn test_many_values() { assert_eq!(tdigest.min_value(), Some(0.0)); assert_eq!(tdigest.max_value(), Some((n - 1) as f64)); - assert_that!(tdigest.rank(0.0).unwrap(), near(0.0, 0.0001)); - assert_that!(tdigest.rank((n / 4) as f64).unwrap(), near(0.25, 0.0001)); - assert_that!(tdigest.rank((n / 2) as f64).unwrap(), near(0.5, 0.0001)); + assert_that!(tdigest.rank(0.0).unwrap().unwrap(), near(0.0, 0.0001)); + assert_that!( + tdigest.rank((n / 4) as f64).unwrap().unwrap(), + near(0.25, 0.0001) + ); + assert_that!( + tdigest.rank((n / 2) as f64).unwrap().unwrap(), + near(0.5, 0.0001) + ); assert_that!( - tdigest.rank((n * 3 / 4) as f64).unwrap(), + tdigest.rank((n * 3 / 4) as f64).unwrap().unwrap(), near(0.75, 0.0001) ); - assert_that!(tdigest.rank(n as f64).unwrap(), eq(1.0)); - assert_that!(tdigest.quantile(0.0).unwrap(), eq(0.0)); + assert_that!(tdigest.rank(n as f64).unwrap().unwrap(), eq(1.0)); + assert_that!(tdigest.quantile(0.0).unwrap().unwrap(), eq(0.0)); assert_that!( - tdigest.quantile(0.5).unwrap(), + tdigest.quantile(0.5).unwrap().unwrap(), near((n / 2) as f64, 0.03 * (n / 2) as f64) ); assert_that!( - tdigest.quantile(0.9).unwrap(), + tdigest.quantile(0.9).unwrap().unwrap(), near((n as f64) * 0.9, 0.01 * (n as f64) * 0.9) ); assert_that!( - tdigest.quantile(0.95).unwrap(), + tdigest.quantile(0.95).unwrap().unwrap(), near((n as f64) * 0.95, 0.01 * (n as f64) * 0.95) ); - assert_that!(tdigest.quantile(1.0).unwrap(), eq((n - 1) as f64)); + assert_that!(tdigest.quantile(1.0).unwrap().unwrap(), eq((n - 1) as f64)); let split_points = [n as f64 / 2.0]; - let pmf = tdigest.pmf(&split_points).unwrap(); + let pmf = tdigest.pmf(&split_points).unwrap().unwrap(); assert_eq!(pmf.len(), 2); assert_that!(pmf[0], near(0.5, 0.0001)); assert_that!(pmf[1], near(0.5, 0.0001)); - let cdf = tdigest.cdf(&split_points).unwrap(); + let cdf = tdigest.cdf(&split_points).unwrap().unwrap(); assert_eq!(cdf.len(), 2); assert_that!(cdf[0], near(0.5, 0.0001)); assert_that!(cdf[1], eq(1.0)); @@ -209,13 +225,13 @@ fn test_rank_two_values() { let mut tdigest = TDigestMut::new(100).unwrap(); tdigest.update(1.0); tdigest.update(2.0); - assert_eq!(tdigest.rank(0.99), Some(0.0)); - assert_eq!(tdigest.rank(1.0), Some(0.25)); - assert_eq!(tdigest.rank(1.25), Some(0.375)); - assert_eq!(tdigest.rank(1.5), Some(0.5)); - assert_eq!(tdigest.rank(1.75), Some(0.625)); - assert_eq!(tdigest.rank(2.0), Some(0.75)); - assert_eq!(tdigest.rank(2.01), Some(1.0)); + assert_eq!(tdigest.rank(0.99).unwrap(), Some(0.0)); + assert_eq!(tdigest.rank(1.0).unwrap(), Some(0.25)); + assert_eq!(tdigest.rank(1.25).unwrap(), Some(0.375)); + assert_eq!(tdigest.rank(1.5).unwrap(), Some(0.5)); + assert_eq!(tdigest.rank(1.75).unwrap(), Some(0.625)); + assert_eq!(tdigest.rank(2.0).unwrap(), Some(0.75)); + assert_eq!(tdigest.rank(2.01).unwrap(), Some(1.0)); } #[test] @@ -225,9 +241,9 @@ fn test_rank_repeated_values() { tdigest.update(1.0); tdigest.update(1.0); tdigest.update(1.0); - assert_eq!(tdigest.rank(0.99), Some(0.0)); - assert_eq!(tdigest.rank(1.0), Some(0.5)); - assert_eq!(tdigest.rank(1.01), Some(1.0)); + assert_eq!(tdigest.rank(0.99).unwrap(), Some(0.0)); + assert_eq!(tdigest.rank(1.0).unwrap(), Some(0.5)); + assert_eq!(tdigest.rank(1.01).unwrap(), Some(1.0)); } #[test] @@ -237,11 +253,11 @@ fn test_repeated_blocks() { tdigest.update(2.0); tdigest.update(2.0); tdigest.update(3.0); - assert_eq!(tdigest.rank(0.99), Some(0.0)); - assert_eq!(tdigest.rank(1.0), Some(0.125)); - assert_eq!(tdigest.rank(2.0), Some(0.5)); - assert_eq!(tdigest.rank(3.0), Some(0.875)); - assert_eq!(tdigest.rank(3.01), Some(1.0)); + assert_eq!(tdigest.rank(0.99).unwrap(), Some(0.0)); + assert_eq!(tdigest.rank(1.0).unwrap(), Some(0.125)); + assert_eq!(tdigest.rank(2.0).unwrap(), Some(0.5)); + assert_eq!(tdigest.rank(3.0).unwrap(), Some(0.875)); + assert_eq!(tdigest.rank(3.01).unwrap(), Some(1.0)); } #[test] @@ -266,13 +282,13 @@ fn test_rank_brackets_weighted_equal_mean_runs() { let expected = [0., 3., 7.5, 18., 32.5, 47., 64.].map(|mass| mass / 64.); let frozen = digest.clone().freeze(); for (&point, &rank) in points.iter().zip(&expected) { - assert_eq!(digest.rank(point), Some(rank)); - assert_eq!(frozen.rank(point), Some(rank)); + assert_eq!(digest.rank(point).unwrap(), Some(rank)); + assert_eq!(frozen.rank(point).unwrap(), Some(rank)); } let mut cdf = expected.to_vec(); cdf.push(1.); - assert_eq!(digest.cdf(&points), Some(cdf.clone())); - assert_eq!(frozen.cdf(&points), Some(cdf)); + assert_eq!(digest.cdf(&points).unwrap(), Some(cdf.clone())); + assert_eq!(frozen.cdf(&points).unwrap(), Some(cdf)); let pmf = vec![ 0., 3. / 64., @@ -283,8 +299,8 @@ fn test_rank_brackets_weighted_equal_mean_runs() { 17. / 64., 0., ]; - assert_eq!(digest.pmf(&points), Some(pmf.clone())); - assert_eq!(frozen.pmf(&points), Some(pmf)); + assert_eq!(digest.pmf(&points).unwrap(), Some(pmf.clone())); + assert_eq!(frozen.pmf(&points).unwrap(), Some(pmf)); } #[test] @@ -299,11 +315,11 @@ fn test_merge_small() { assert_eq!(td1.min_value(), Some(1.0)); assert_eq!(td1.max_value(), Some(3.0)); assert_eq!(td1.total_weight(), 4); - assert_eq!(td1.rank(0.99), Some(0.0)); - assert_eq!(td1.rank(1.0), Some(0.125)); - assert_eq!(td1.rank(2.0), Some(0.5)); - assert_eq!(td1.rank(3.0), Some(0.875)); - assert_eq!(td1.rank(3.01), Some(1.0)); + assert_eq!(td1.rank(0.99).unwrap(), Some(0.0)); + assert_eq!(td1.rank(1.0).unwrap(), Some(0.125)); + assert_eq!(td1.rank(2.0).unwrap(), Some(0.5)); + assert_eq!(td1.rank(3.0).unwrap(), Some(0.875)); + assert_eq!(td1.rank(3.01).unwrap(), Some(1.0)); } #[test] @@ -323,11 +339,20 @@ fn test_merge_large() { assert_eq!(td1.min_value(), Some(0.0)); assert_eq!(td1.max_value(), Some((n - 1) as f64)); - assert_that!(td1.rank(0.0).unwrap(), near(0.0, 0.0001)); - assert_that!(td1.rank((n / 4) as f64).unwrap(), near(0.25, 0.0001)); - assert_that!(td1.rank((n / 2) as f64).unwrap(), near(0.5, 0.0001)); - assert_that!(td1.rank((n * 3 / 4) as f64).unwrap(), near(0.75, 0.0001)); - assert_that!(td1.rank(n as f64).unwrap(), eq(1.0)); + assert_that!(td1.rank(0.0).unwrap().unwrap(), near(0.0, 0.0001)); + assert_that!( + td1.rank((n / 4) as f64).unwrap().unwrap(), + near(0.25, 0.0001) + ); + assert_that!( + td1.rank((n / 2) as f64).unwrap().unwrap(), + near(0.5, 0.0001) + ); + assert_that!( + td1.rank((n * 3 / 4) as f64).unwrap().unwrap(), + near(0.75, 0.0001) + ); + assert_that!(td1.rank(n as f64).unwrap().unwrap(), eq(1.0)); } #[test] @@ -346,8 +371,11 @@ fn test_mixed_k_merge_retains_receiver_k() { assert_eq!(left.k(), left_k); assert_eq!(left.total_weight(), 10_000 + right_count); - assert_eq!(left.quantile(0.0), Some(0.0)); - assert_eq!(left.quantile(1.0), Some((9_999 + right_count) as f64)); + assert_eq!(left.quantile(0.0).unwrap(), Some(0.0)); + assert_eq!( + left.quantile(1.0).unwrap(), + Some((9_999 + right_count) as f64) + ); } } @@ -360,8 +388,8 @@ fn test_from_iter_uses_one_result_with_the_smallest_nonempty_k() { first.update(value as f64); second.update((value + 1_000) as f64); } - let _ = first.quantile(0.5); - let _ = second.quantile(0.5); + let _ = first.quantile(0.5).unwrap(); + let _ = second.quantile(0.5).unwrap(); let mut merged = [first, empty, second].into_iter().collect::(); @@ -370,7 +398,7 @@ fn test_from_iter_uses_one_result_with_the_smallest_nonempty_k() { assert_eq!(merged.min_value(), Some(0.0)); assert_eq!(merged.max_value(), Some(1_999.0)); let quantiles = (0..=100) - .map(|rank| merged.quantile(rank as f64 / 100.).unwrap()) + .map(|rank| merged.quantile(rank as f64 / 100.).unwrap().unwrap()) .collect::>(); assert!(quantiles.windows(2).all(|pair| pair[0] <= pair[1])); } @@ -431,8 +459,8 @@ fn test_from_iter_matches_single_compression_for_interleaved_runs() { let mut merged = runs.into_iter().map(make_digest).collect::(); assert_eq!(merged.serialize(), expected.serialize()); - assert_eq!(merged.quantile(0.0), Some(-10.0)); - assert_eq!(merged.quantile(1.0), Some(20.0)); + assert_eq!(merged.quantile(0.0).unwrap(), Some(-10.0)); + assert_eq!(merged.quantile(1.0).unwrap(), Some(20.0)); // A collected digest must still support enough updates to grow and compress again. for value in 0..1_000 { @@ -455,11 +483,11 @@ fn test_from_iter_handles_mixed_compressed_and_buffered_inputs() { digest.update(value); expected.update(value); if run == 2 && row == 29 { - let _ = digest.quantile(0.5); + let _ = digest.quantile(0.5).unwrap(); } } if run == 1 { - let _ = digest.quantile(0.5); + let _ = digest.quantile(0.5).unwrap(); } digest }) @@ -506,12 +534,12 @@ fn test_serialized_batch_merge_tree_preserves_weight_and_quantiles() { let mut merged = TDigestMut::deserialize(&states[0]).unwrap(); assert_eq!(merged.total_weight(), values.len() as u64); - assert_eq!(merged.quantile(0.0), values.first().copied()); - assert_eq!(merged.quantile(1.0), values.last().copied()); + assert_eq!(merged.quantile(0.0).unwrap(), values.first().copied()); + assert_eq!(merged.quantile(1.0).unwrap(), values.last().copied()); let mut previous = values[0]; for rank in [0.01, 0.5, 0.9, 0.99] { - let estimate = merged.quantile(rank).unwrap(); + let estimate = merged.quantile(rank).unwrap().unwrap(); assert!((previous..=values[values.len() - 1]).contains(&estimate)); // Check this fixture's empirical rank, allowing equal values to span a rank interval. let lower = @@ -535,7 +563,7 @@ fn test_from_iter_checks_total_weight_before_compression() { let mut digest = TDigestMut::new(100).unwrap(); digest.update(value); if compressed { - let _ = digest.quantile(0.5); + let _ = digest.quantile(0.5).unwrap(); } digest }); @@ -543,8 +571,8 @@ fn test_from_iter_checks_total_weight_before_compression() { .into_iter() .collect::(); assert_eq!(at_limit.total_weight(), u64::MAX); - assert_eq!(at_limit.quantile(0.0), Some(0.0)); - assert_eq!(at_limit.quantile(1.0), Some(2.0)); + assert_eq!(at_limit.quantile(0.0).unwrap(), Some(0.0)); + assert_eq!(at_limit.quantile(1.0).unwrap(), Some(2.0)); assert!( catch_unwind(|| [heavy, one, two, extra].into_iter().collect::()).is_err() @@ -614,7 +642,7 @@ fn test_extreme_values_produce_finite_quantiles() { assert_eq!(tdigest.min_value(), Some(-f64::MAX)); assert_eq!(tdigest.max_value(), Some(f64::MAX)); for rank in [0.25, 0.5, 0.75] { - let quantile = tdigest.quantile(rank).unwrap(); + let quantile = tdigest.quantile(rank).unwrap().unwrap(); assert_that!(quantile, is_finite(), "quantile at rank {rank}"); } } @@ -630,7 +658,7 @@ fn test_rank_interpolation_across_extreme_finite_values() { let points = [-scale, -scale / 2., 0., scale / 2., scale]; for (point, expected) in points.into_iter().zip([0.25, 0.375, 0.5, 0.625, 0.75]) { assert_that!( - digest.rank(point).unwrap(), + digest.rank(point).unwrap().unwrap(), near(expected, 4. * f64::EPSILON) ); } @@ -647,11 +675,11 @@ fn test_tail_interpolation_across_extreme_finite_values() { for (digest, rank, expected) in [(left, 3. / 11., -0.25), (right, 8. / 11., 0.25)] { let digest = digest.freeze(); assert_that!( - digest.quantile(rank).unwrap() / max, + digest.quantile(rank).unwrap().unwrap() / max, near(expected, 4. * f64::EPSILON) ); assert_that!( - digest.rank(expected * max).unwrap(), + digest.rank(expected * max).unwrap().unwrap(), near(rank, 4. * f64::EPSILON) ); } @@ -663,7 +691,7 @@ fn test_estimate_repeat_values() { for _ in 0..20 { tdigest.update(1.0); } - assert_eq!(tdigest.quantile(0.9), Some(1.0)); + assert_eq!(tdigest.quantile(0.9).unwrap(), Some(1.0)); } /// Builds a weighted image, including tail centroids unavailable through raw updates. @@ -690,8 +718,8 @@ fn deserialize_with_centroids(k: u16, min: f64, max: f64, centroids: &[(f64, u64 fn assert_quantile_queries(tdigest: &TDigestMut, ranks: &[f64], expected: &[f64]) { let frozen = tdigest.clone().freeze(); for actual in [ - tdigest.clone().quantiles(ranks).unwrap(), - frozen.quantiles(ranks).unwrap(), + tdigest.clone().quantiles(ranks).unwrap().unwrap(), + frozen.quantiles(ranks).unwrap().unwrap(), ] { assert_eq!(actual.len(), expected.len()); for (actual, &expected) in actual.into_iter().zip(expected) { @@ -700,8 +728,14 @@ fn assert_quantile_queries(tdigest: &TDigestMut, ranks: &[f64], expected: &[f64] } let mut mutable = tdigest.clone(); for (&rank, &expected) in ranks.iter().zip(expected) { - assert_that!(mutable.quantile(rank).unwrap(), near(expected, 1e-12)); - assert_that!(frozen.quantile(rank).unwrap(), near(expected, 1e-12)); + assert_that!( + mutable.quantile(rank).unwrap().unwrap(), + near(expected, 1e-12) + ); + assert_that!( + frozen.quantile(rank).unwrap().unwrap(), + near(expected, 1e-12) + ); } } @@ -712,11 +746,26 @@ fn test_quantile_moves_toward_the_nearer_bracketing_centroid() { assert_eq!(tdigest.total_weight(), 12); // Ranks 2/12 and 6/12 sit exactly on the two centroids bracketing the first interval. - assert_that!(tdigest.quantile(2.0 / 12.0).unwrap(), near(0.0, 1e-12)); - assert_that!(tdigest.quantile(3.0 / 12.0).unwrap(), near(2.5, 1e-12)); - assert_that!(tdigest.quantile(4.0 / 12.0).unwrap(), near(5.0, 1e-12)); - assert_that!(tdigest.quantile(5.0 / 12.0).unwrap(), near(7.5, 1e-12)); - assert_that!(tdigest.quantile(6.0 / 12.0).unwrap(), near(10.0, 1e-12)); + assert_that!( + tdigest.quantile(2.0 / 12.0).unwrap().unwrap(), + near(0.0, 1e-12) + ); + assert_that!( + tdigest.quantile(3.0 / 12.0).unwrap().unwrap(), + near(2.5, 1e-12) + ); + assert_that!( + tdigest.quantile(4.0 / 12.0).unwrap().unwrap(), + near(5.0, 1e-12) + ); + assert_that!( + tdigest.quantile(5.0 / 12.0).unwrap().unwrap(), + near(7.5, 1e-12) + ); + assert_that!( + tdigest.quantile(6.0 / 12.0).unwrap().unwrap(), + near(10.0, 1e-12) + ); } #[test] @@ -725,11 +774,20 @@ fn test_quantile_right_tail_stays_within_max() { deserialize_with_centroids(100, 0.0, 100.0, &[(10.0, 10), (50.0, 10), (90.0, 10)]); assert_eq!(tdigest.max_value(), Some(100.0)); - assert_that!(tdigest.quantile(0.9).unwrap(), near(95.0, 1e-12)); - assert_that!(tdigest.quantile(29.0 / 30.0).unwrap(), near(100.0, 1e-12)); + assert_that!(tdigest.quantile(0.9).unwrap().unwrap(), near(95.0, 1e-12)); + assert_that!( + tdigest.quantile(29.0 / 30.0).unwrap().unwrap(), + near(100.0, 1e-12) + ); // Mirrors the left tail, which interpolates from min up to the first centroid mean. - assert_that!(tdigest.quantile(1.0 / 30.0).unwrap(), near(0.0, 1e-12)); - assert_that!(tdigest.quantile(5.0 / 30.0).unwrap(), near(10.0, 1e-12)); + assert_that!( + tdigest.quantile(1.0 / 30.0).unwrap().unwrap(), + near(0.0, 1e-12) + ); + assert_that!( + tdigest.quantile(5.0 / 30.0).unwrap().unwrap(), + near(10.0, 1e-12) + ); } #[test] @@ -737,7 +795,7 @@ fn test_quantile_handles_two_sample_last_centroid() { let mut tdigest = deserialize_with_centroids(100, 0.0, 100.0, &[(0.0, 1), (50.0, 1), (90.0, 2)]); - assert_eq!(tdigest.quantile(0.75), Some(100.0)); + assert_eq!(tdigest.quantile(0.75).unwrap(), Some(100.0)); } #[test] @@ -757,10 +815,13 @@ fn test_single_centroid_preserves_stored_tail_information() { // Repeated queries agree before and after creating a mutable view. for _ in 0..2 { for (&point, &rank) in points.iter().zip(&expected) { - assert_eq!(digest.rank(point), Some(rank)); - assert_eq!(frozen.rank(point), Some(rank)); + assert_eq!(digest.rank(point).unwrap(), Some(rank)); + assert_eq!(frozen.rank(point).unwrap(), Some(rank)); } - assert_eq!(&digest.cdf(&points).unwrap()[..points.len()], &expected); + assert_eq!( + &digest.cdf(&points).unwrap().unwrap()[..points.len()], + &expected + ); } } @@ -786,14 +847,14 @@ fn test_rank_tails_stay_monotonic_after_updating_weighted_centroids() { // The new unit-weight tails lie inside the retained extrema. Their half-sample // centers leave no mass for interpolation between min/first or last/max. for (&point, &expected) in points.iter().zip(&expected) { - assert_eq!(digest.rank(point), Some(expected)); - assert_eq!(frozen.rank(point), Some(expected)); + assert_eq!(digest.rank(point).unwrap(), Some(expected)); + assert_eq!(frozen.rank(point).unwrap(), Some(expected)); } - let cdf = digest.cdf(&points).unwrap(); - assert_eq!(cdf, frozen.cdf(&points).unwrap()); + let cdf = digest.cdf(&points).unwrap().unwrap(); + assert_eq!(cdf, frozen.cdf(&points).unwrap().unwrap()); assert!(cdf.is_sorted(), "CDF at {points:?}: {cdf:?}"); - let pmf = digest.pmf(&points).unwrap(); - assert_eq!(pmf, frozen.pmf(&points).unwrap()); + let pmf = digest.pmf(&points).unwrap().unwrap(); + assert_eq!(pmf, frozen.pmf(&points).unwrap().unwrap()); assert!(pmf.iter().all(|&mass| mass >= 0.), "PMF: {pmf:?}"); assert_that!(pmf.iter().sum::(), near(1., 1e-12)); } @@ -992,11 +1053,11 @@ fn test_signed_zero_and_symmetric_centroids_survive_interpolation() { let mut digest = deserialize_with_centroids(100, -1., 1., &[(-1., 1), (-0., 2), (0., 2), (1., 1)]); - assert_eq!(digest.rank(-0.), Some(0.5)); - assert_eq!(digest.rank(0.), Some(0.5)); + assert_eq!(digest.rank(-0.).unwrap(), Some(0.5)); + assert_eq!(digest.rank(0.).unwrap(), Some(0.5)); assert_quantile_queries(&digest, &[1. / 3., 0.5, 2. / 3.], &[0., 0., 0.]); - assert_eq!(digest.cdf(&[-0.]), Some(vec![0.5, 1.])); - assert_eq!(digest.pmf(&[0.]), Some(vec![0.5, 0.5])); + assert_eq!(digest.cdf(&[-0.]).unwrap(), Some(vec![0.5, 1.])); + assert_eq!(digest.pmf(&[0.]).unwrap(), Some(vec![0.5, 0.5])); } #[test] @@ -1015,11 +1076,14 @@ fn test_batch_quantiles_match_scalar_queries_in_input_order() { ] { let expected = ranks .iter() - .map(|&rank| frozen.quantile(rank).unwrap()) + .map(|&rank| frozen.quantile(rank).unwrap().unwrap()) .collect::>(); // Each mutable batch starts with the original buffered values still pending. - assert_eq!(tdigest.clone().quantiles(&ranks), Some(expected.clone())); - assert_eq!(frozen.quantiles(&ranks), Some(expected)); + assert_eq!( + tdigest.clone().quantiles(&ranks).unwrap(), + Some(expected.clone()) + ); + assert_eq!(frozen.quantiles(&ranks).unwrap(), Some(expected)); } } @@ -1109,9 +1173,11 @@ fn test_quantiles_stay_within_extrema_at_large_total_weights() { let frozen = tdigest.clone().freeze(); let ranks = [0.0, 0.5, 1.0_f64.next_down(), 1.0, 1.0]; for quantiles in [ - tdigest.clone().quantiles(&ranks).unwrap(), - frozen.quantiles(&ranks).unwrap(), - ranks.map(|rank| frozen.quantile(rank).unwrap()).to_vec(), + tdigest.clone().quantiles(&ranks).unwrap().unwrap(), + frozen.quantiles(&ranks).unwrap().unwrap(), + ranks + .map(|rank| frozen.quantile(rank).unwrap().unwrap()) + .to_vec(), ] { assert_eq!(quantiles[0], 0.0); assert_eq!(&quantiles[3..], &[100.0, 100.0]); @@ -1144,7 +1210,7 @@ fn test_queries_stay_ordered_at_large_centroid_centers() { 1.0_f64.next_down(), 1., ]; - let quantiles = digest.quantiles(&ranks).unwrap(); + let quantiles = digest.quantiles(&ranks).unwrap().unwrap(); assert_eq!(quantiles[0], 0.); assert_eq!(quantiles[5], 100.); assert!( @@ -1153,7 +1219,10 @@ fn test_queries_stay_ordered_at_large_centroid_centers() { ); assert!(quantiles.iter().all(|value| (0.0..=100.0).contains(value))); - let cdf = digest.cdf(&[0., 5., 10., 50., 90., 95., 100.]).unwrap(); + let cdf = digest + .cdf(&[0., 5., 10., 50., 90., 95., 100.]) + .unwrap() + .unwrap(); assert!(cdf.is_sorted(), "centroids {centroids:?}: {cdf:?}"); assert!(cdf.iter().all(|rank| (0.0..=1.0).contains(rank))); } @@ -1176,7 +1245,7 @@ fn test_quantile_right_tail_uses_the_same_center_as_rank() { ); // These ranks land exactly on the rounded mass N - last_weight/2. Quantile must // return the centroid mean there, before interpolating toward the stored maximum. - assert_eq!(digest.rank(90.), Some(rank)); + assert_eq!(digest.rank(90.).unwrap(), Some(rank)); assert_quantile_queries(&digest, &[rank, rank, 1.], &[90., 90., 100.]); } @@ -1186,7 +1255,7 @@ fn test_quantile_right_tail_uses_the_same_center_as_rank() { let digest = deserialize_with_centroids(100, 0., 100., &[(0., total_weight - 3), (90., 3)]); let rank = 1. - f64::EPSILON; let ranks = [rank.next_down(), rank, rank.next_up(), 1.]; - let quantiles = digest.clone().quantiles(&ranks).unwrap(); + let quantiles = digest.clone().quantiles(&ranks).unwrap().unwrap(); assert!(quantiles.is_sorted()); assert!(quantiles.iter().all(|value| (0.0..=100.0).contains(value))); assert_eq!(quantiles[3], 100.); @@ -1227,10 +1296,17 @@ fn test_rank_max_uses_the_same_rounded_mass_as_last_centroid() { deserialize_with_centroids(100, 0., 100., &[(0., total_weight - 1), (90., 1)]); let frozen = digest.clone().freeze(); let points = [90., 90_f64.next_up(), 95., 100_f64.next_down(), 100.]; - let cdf = digest.cdf(&points).unwrap(); + let cdf = digest.cdf(&points).unwrap().unwrap(); assert_eq!(&cdf[..points.len()], &[expected; 5]); - assert_eq!(cdf, frozen.cdf(&points).unwrap()); - assert!(frozen.pmf(&points).unwrap().iter().all(|&mass| mass >= 0.)); + assert_eq!(cdf, frozen.cdf(&points).unwrap().unwrap()); + assert!( + frozen + .pmf(&points) + .unwrap() + .unwrap() + .iter() + .all(|&mass| mass >= 0.) + ); } } @@ -1238,20 +1314,28 @@ fn test_rank_max_uses_the_same_rounded_mass_as_last_centroid() { fn test_rank_stays_monotonic_at_the_right_tail_boundary() { let digest = deserialize_with_centroids(100, 0., 300., &[(0., 1), (100., 29)]).freeze(); let points = [100., 100_f64.next_up(), 150., 300.]; - let cdf = digest.cdf(&points).unwrap(); + let cdf = digest.cdf(&points).unwrap().unwrap(); // The last center is at weight 1 + 29/2 = 15.5. Entering the tail must not lower its rank. assert_eq!(cdf[0], 15.5 / 30.); assert!(cdf.is_sorted(), "CDF at {points:?}: {cdf:?}"); } #[test] -fn test_batch_quantiles_reject_invalid_ranks() { - for values in [&[][..], &[1.0, 2.0, 3.0][..]] { +fn test_invalid_queries_return_errors_before_checking_emptiness() { + for n in [0, 1, 1_000] { let mut tdigest = TDigestMut::default(); - for &value in values { - tdigest.update(value); + for value in 0..n { + tdigest.update(f64::from(value)); } let frozen = tdigest.clone().freeze(); + assert_eq!( + tdigest.rank(f64::NAN).unwrap_err().kind(), + ErrorKind::InvalidArgument + ); + assert_eq!( + frozen.rank(f64::NAN).unwrap_err().kind(), + ErrorKind::InvalidArgument + ); for invalid in [ -f64::EPSILON, 1.0 + f64::EPSILON, @@ -1260,24 +1344,77 @@ fn test_batch_quantiles_reject_invalid_ranks() { f64::INFINITY, ] { let ranks = [0.5, invalid]; - assert!(catch_unwind(AssertUnwindSafe(|| tdigest.quantiles(&ranks))).is_err()); - assert!(catch_unwind(|| frozen.quantiles(&ranks)).is_err()); + for error in [ + tdigest.quantile(invalid).unwrap_err(), + frozen.quantile(invalid).unwrap_err(), + tdigest.quantiles(&ranks).unwrap_err(), + frozen.quantiles(&ranks).unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } + } + for split_points in [ + &[f64::NAN][..], + &[0.0, f64::NAN], + &[f64::NAN, 0.0], + &[1.0, 1.0], + &[2.0, 1.0], + &[-0.0, 0.0], + &[f64::INFINITY, f64::INFINITY], + ] { + for error in [ + tdigest.cdf(split_points).unwrap_err(), + frozen.cdf(split_points).unwrap_err(), + tdigest.pmf(split_points).unwrap_err(), + frozen.pmf(split_points).unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::InvalidArgument); + } } } } +#[test] +fn test_infinite_values_are_valid_rank_and_split_point_arguments() { + for n in [0, 1, 1_000] { + let mut tdigest = TDigestMut::default(); + for value in 0..n { + tdigest.update(f64::from(value)); + } + let frozen = tdigest.clone().freeze(); + for (value, rank) in [(f64::NEG_INFINITY, 0.0), (f64::INFINITY, 1.0)] { + let expected = (n > 0).then_some(rank); + assert_eq!(tdigest.rank(value).unwrap(), expected); + assert_eq!(frozen.rank(value).unwrap(), expected); + } + let split_points = [f64::NEG_INFINITY, f64::INFINITY]; + let cdf = (n > 0).then(|| vec![0.0, 1.0, 1.0]); + let pmf = (n > 0).then(|| vec![0.0, 1.0, 0.0]); + assert_eq!(tdigest.cdf(&split_points).unwrap(), cdf); + assert_eq!(frozen.cdf(&split_points).unwrap(), cdf); + assert_eq!(tdigest.pmf(&split_points).unwrap(), pmf); + assert_eq!(frozen.pmf(&split_points).unwrap(), pmf); + } +} + #[test] fn test_rank_left_tail_is_a_fraction_of_the_total_weight() { let mut tdigest = deserialize_with_centroids(100, 0.0, 100.0, &[(10.0, 10), (50.0, 10), (90.0, 10)]); - assert_that!(tdigest.rank(5.0).unwrap(), near(0.1, 1e-12)); - assert_that!(tdigest.rank(10.0).unwrap(), near(5.0 / 30.0, 1e-12)); + assert_that!(tdigest.rank(5.0).unwrap().unwrap(), near(0.1, 1e-12)); + assert_that!( + tdigest.rank(10.0).unwrap().unwrap(), + near(5.0 / 30.0, 1e-12) + ); // The right tail is the mirror image and pins the scale the left tail must match. - assert_that!(tdigest.rank(95.0).unwrap(), near(0.9, 1e-12)); - assert_that!(tdigest.rank(90.0).unwrap(), near(25.0 / 30.0, 1e-12)); + assert_that!(tdigest.rank(95.0).unwrap().unwrap(), near(0.9, 1e-12)); + assert_that!( + tdigest.rank(90.0).unwrap().unwrap(), + near(25.0 / 30.0, 1e-12) + ); - let pmf = tdigest.pmf(&[5.0, 95.0]).unwrap(); + let pmf = tdigest.pmf(&[5.0, 95.0]).unwrap().unwrap(); assert_that!(pmf[0], near(0.1, 1e-12)); assert_that!(pmf[1], near(0.8, 1e-12)); assert_that!(pmf[2], near(0.1, 1e-12)); @@ -1295,20 +1432,20 @@ fn test_merge_preserves_min_max_from_other() { empty.merge(&other); assert_eq!(empty.min_value(), Some(0.0)); assert_eq!(empty.max_value(), Some(100.0)); - assert_eq!(empty.quantile(0.0), Some(0.0)); - assert_eq!(empty.quantile(1.0), Some(100.0)); - assert_eq!(empty.rank(0.0), Some(0.5 / 30.0)); - assert_eq!(empty.rank(100.0), Some(1.0 - 0.5 / 30.0)); - assert_eq!(empty.rank(-1.0), Some(0.0)); - assert_eq!(empty.rank(101.0), Some(1.0)); + assert_eq!(empty.quantile(0.0).unwrap(), Some(0.0)); + assert_eq!(empty.quantile(1.0).unwrap(), Some(100.0)); + assert_eq!(empty.rank(0.0).unwrap(), Some(0.5 / 30.0)); + assert_eq!(empty.rank(100.0).unwrap(), Some(1.0 - 0.5 / 30.0)); + assert_eq!(empty.rank(-1.0).unwrap(), Some(0.0)); + assert_eq!(empty.rank(101.0).unwrap(), Some(1.0)); let mut left = deserialize_with_centroids(100, 5.0, 40.0, &[(10.0, 4), (20.0, 4), (30.0, 4)]); let right = deserialize_with_centroids(100, -10.0, 80.0, &[(0.0, 4), (40.0, 4), (70.0, 4)]); left.merge(&right); assert_eq!(left.min_value(), Some(-10.0)); assert_eq!(left.max_value(), Some(80.0)); - assert_eq!(left.quantile(0.0), Some(-10.0)); - assert_eq!(left.quantile(1.0), Some(80.0)); + assert_eq!(left.quantile(0.0).unwrap(), Some(-10.0)); + assert_eq!(left.quantile(1.0).unwrap(), Some(80.0)); } #[test]