Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Quantile Remapping Fix #304

Merged
merged 3 commits into from
May 30, 2022
Merged

Quantile Remapping Fix #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9cfd1f4
Upgrade DDSketch to 0.1.3
May 27, 2022
d215b30
Return self.min()/max() for the 0.0/1.0 quantiles
May 27, 2022
5274708
Fix rank calculation in Summary::quantile()
May 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions metrics-util/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ aho-corasick = { version = "0.7", default-features = false, optional = true, fea
indexmap = { version = "1", default-features = false, optional = true }
parking_lot = { version = "0.11", default-features = false, optional = true }
quanta = { version = "0.9.3", default-features = false, optional = true }
sketches-ddsketch = { version = "0.1", default-features = false, optional = true }
sketches-ddsketch = { version = "0.1.3", default-features = false, optional = true }
radix_trie = { version = "0.2", default-features = false, optional = true }
ordered-float = { version = "2.0", default-features = false, optional = true }
num_cpus = { version = "1", default-features = false, optional = true }
Expand All @@ -68,7 +68,7 @@ rand = { version = "0.8", features = ["small_rng"] }
rand_distr = "0.4"
getopts = "0.2"
hdrhistogram = { version = "7.2", default-features = false }
sketches-ddsketch = "0.1"
sketches-ddsketch = "0.1.3"
ndarray = "0.15"
ndarray-stats = "0.5"
noisy_float = "0.2"
Expand Down
73 changes: 54 additions & 19 deletions metrics-util/src/summary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,29 +122,33 @@ impl Summary {
/// If the sketch is empty, or if the quantile is less than 0.0 or greater than 1.0, then the
/// result will be `None`.
///
/// While `q` can be either 0.0 or 1.0, callers should prefer to use [`Summary::min`] and
/// [`Summary::max`] as the values will be the true values, and not an estimation.
/// If the 0.0 or 1.0 quantile is requested, this function will return self.min() or self.max()
/// instead of the estimated value.
pub fn quantile(&self, q: f64) -> Option<f64> {
if !(0.0..=1.0).contains(&q) || self.count() == 0 {
return None;
} else if q == 0.0 {
return Some(self.min());
} else if q == 1.0 {
return Some(self.max());
}

let ncount = self.negative.count();
let pcount = self.positive.count();
let zcount = self.zeroes;
let total = ncount + pcount + zcount;
let rank = (q * (total - 1) as f64) as usize;
let ncount = self.negative.count() as f64;
let pcount = self.positive.count() as f64;
let zcount = self.zeroes as f64;
// Defer rounding to the underlying sketch
let rank = q * (ncount + pcount + zcount);

if rank < ncount {
if rank <= ncount {
// Quantile lands in the negative side.
let nq = 1.0 - (rank as f64 / ncount as f64);
let nq = 1.0 - (rank / ncount as f64);
self.negative.quantile(nq).expect("quantile should be valid at this point").map(|v| -v)
} else if rank >= ncount && rank < (ncount + zcount) {
} else if rank <= (ncount + zcount) {
// Quantile lands in the zero band.
Some(0.0)
} else {
// Quantile lands in the positive side.
let pq = (rank - (ncount + zcount)) as f64 / pcount as f64;
let pq = (rank - (ncount + zcount)) / pcount;
self.positive.quantile(pq).expect("quantile should be valid at this point")
}
}
Expand Down Expand Up @@ -202,31 +206,62 @@ mod tests {

#[test]
fn test_basics() {
let mut summary = Summary::with_defaults();
let alpha = 0.0001;
let max_bins = 32_768;
let min_value = 1.0e-9;
let mut summary = Summary::new(alpha, max_bins, min_value);
assert!(summary.is_empty());

// Stretch the legs with a single value.
summary.add(-420.42);
assert_eq!(summary.count(), 1);
assert_relative_eq!(summary.min(), -420.42);
assert_relative_eq!(summary.max(), -420.42);
assert_abs_diff_eq!(summary.quantile(0.1).expect("value should exist"), -420.42);
assert_abs_diff_eq!(summary.quantile(0.5).expect("value should exist"), -420.42);
assert_abs_diff_eq!(summary.quantile(0.99).expect("value should exist"), -420.42);
let alpha = 0.001;

let test_cases = vec![(0.1, -420.42), (0.5, -420.42), (0.9, -420.42)];
for (q, val) in test_cases {
assert_relative_eq!(
summary.quantile(q).expect("value should exist"),
val,
max_relative = 2.0 * alpha * val
);
}

summary.add(420.42);
assert_eq!(summary.count(), 2);
assert_relative_eq!(summary.min(), -420.42);
assert_relative_eq!(summary.max(), 420.42);
assert_abs_diff_eq!(summary.quantile(0.49).expect("value should exist"), -420.42);
assert_relative_eq!(
summary.quantile(0.5).expect("value should exist"),
-420.42,
max_relative = 2.0 * alpha * 420.42
);
assert_relative_eq!(
summary.quantile(0.51).expect("value should exist"),
420.42,
max_relative = 2.0 * alpha * 420.42
);

summary.add(42.42);
assert_eq!(summary.count(), 3);
assert_relative_eq!(summary.min(), -420.42);
assert_relative_eq!(summary.max(), 420.42);
assert_abs_diff_eq!(summary.quantile(0.4999999999).expect("value should exist"), -420.42);
assert_abs_diff_eq!(summary.quantile(0.5).expect("value should exist"), 42.42);
assert_abs_diff_eq!(summary.quantile(0.9999999999).expect("value should exist"), 42.42);

let test_cases = vec![
(0.333333, -420.42),
(0.333334, 42.42),
(0.666666, 42.42),
(0.666667, 420.42),
(0.999999, 420.42),
];
for (q, val) in test_cases {
assert_relative_eq!(
summary.quantile(q).expect("value should exist"),
val,
max_relative = 2.0 * alpha * val
);
}
}

#[test]
Expand Down