From f331ce500455fc8b07dfbf16f2220f741aaf0507 Mon Sep 17 00:00:00 2001 From: Adam Fontenot Date: Sun, 7 May 2023 21:40:26 -0400 Subject: [PATCH] add tests for exponential weighted average --- src/state.rs | 120 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 87 insertions(+), 33 deletions(-) diff --git a/src/state.rs b/src/state.rs index 7148a7c4..553bdde0 100644 --- a/src/state.rs +++ b/src/state.rs @@ -70,7 +70,7 @@ impl BarState { pub(crate) fn reset(&mut self, now: Instant, mode: Reset) { if let Reset::Eta | Reset::All = mode { - self.state.est.reset(); + self.state.est.reset(now); } if let Reset::Elapsed | Reset::All = mode { @@ -224,13 +224,14 @@ pub struct ProgressState { impl ProgressState { pub(crate) fn new(len: Option, pos: Arc) -> Self { + let now = Instant::now(); Self { pos, len, tick: 0, status: Status::InProgress, - started: Instant::now(), - est: Estimator::new(), + started: now, + est: Estimator::new(now), message: TabExpandedString::NoTabs("".into()), prefix: TabExpandedString::NoTabs("".into()), } @@ -270,7 +271,7 @@ impl ProgressState { let pos = self.pos.pos.load(Ordering::Relaxed); - let sps = self.est.steps_per_second(); + let sps = self.est.steps_per_second(Instant::now()); // Infinite duration should only ever happen at the beginning, so in this case it's okay to // just show an ETA of 0 until progress starts to occur. @@ -292,7 +293,7 @@ impl ProgressState { /// The number of steps per second pub fn per_sec(&self) -> f64 { if let Status::InProgress = self.status { - self.est.steps_per_second() + self.est.steps_per_second(Instant::now()) } else { let len = self.len.unwrap_or_else(|| self.pos()); len as f64 / self.started.elapsed().as_secs_f64() @@ -387,8 +388,7 @@ pub(crate) struct Estimator { } impl Estimator { - fn new() -> Self { - let now = Instant::now(); + fn new(now: Instant) -> Self { Self { smoothed_steps_per_sec: 0.0, double_smoothed_steps_per_sec: 0.0, @@ -417,7 +417,8 @@ impl Estimator { // Reset on backwards seek to prevent breakage from seeking to the end for length determination // See https://github.com/console-rs/indicatif/issues/480 if new < self.prev.0 { - self.reset(); + self.prev.0 = new; + self.reset(now); } return; } @@ -445,8 +446,9 @@ impl Estimator { self.prev = (new, now); } - pub(crate) fn reset(&mut self) { - let now = Instant::now(); + /// Reset the state of the estimator. Once reset, estimates will not depend on any data prior + /// to `now`. This does not reset the position of the progress bar. + pub(crate) fn reset(&mut self, now: Instant) { self.smoothed_steps_per_sec = 0.0; self.double_smoothed_steps_per_sec = 0.0; self.prev = (self.prev.0, now); @@ -461,9 +463,7 @@ impl Estimator { } /// Average time per step in seconds, using single or double exponential smoothing - fn steps_per_second(&self) -> f64 { - let now = Instant::now(); - + fn steps_per_second(&self, now: Instant) -> f64 { // reweight to account for the passage of time since last tick let delta_t = duration_to_secs(now - self.prev.1); let reweight = self.weight(delta_t); @@ -630,29 +630,29 @@ mod tests { // https://github.com/rust-lang/rust-clippy/issues/10281 #[allow(clippy::uninlined_format_args)] #[test] - fn test_time_per_step() { + fn test_steps_per_second() { let test_rate = |items_per_second| { let mut now = Instant::now(); let mut est = Estimator::new(now); let mut pos = 0; - for _ in 0..est.steps.len() { + for _ in 0..20 { pos += items_per_second; now += Duration::from_secs(1); est.record(pos, now); } - let avg_seconds_per_step = est.seconds_per_step(); + let avg_steps_per_second = est.steps_per_second(now); - assert!(avg_seconds_per_step > 0.0); - assert!(avg_seconds_per_step.is_finite()); + assert!(avg_steps_per_second > 0.0); + assert!(avg_steps_per_second.is_finite()); - let expected_rate = 1.0 / items_per_second as f64; - let absolute_error = (avg_seconds_per_step - expected_rate).abs(); + let absolute_error = (avg_steps_per_second - items_per_second as f64).abs(); + let relative_error = absolute_error / items_per_second as f64; assert!( - absolute_error < f64::EPSILON, + relative_error < 0.000000001, // one part per billion "Expected rate: {}, actual: {}, absolute error: {}", - expected_rate, - avg_seconds_per_step, + items_per_second, + avg_steps_per_second, absolute_error ); }; @@ -669,24 +669,55 @@ mod tests { } #[test] - fn test_duration_stuff() { - let duration = Duration::new(42, 100_000_000); - let secs = duration_to_secs(duration); - assert_eq!(secs_to_duration(secs), duration); + fn test_single_double_exponential_ave() { + let mut now = Instant::now(); + let mut est = Estimator::new(now); + let mut pos = 0; + + // note: this is the default weight set in the Estimator + let weight = 15; + + for _ in 0..weight { + pos += 1; + now += Duration::from_secs(1); + est.record(pos, now); + } + now += Duration::from_secs(weight); + + // first test with only a single EWA + est.disable_double_smoothing(); + + // 90% weight @ 0 eps, 9% weight @ 1 eps, 1% weight @ 0 eps + // then debiased by deweighting the 1% weight (before -30 seconds) + let target = 0.09 / 0.99; + assert_eq!(est.steps_per_second(now), target); + + // now test with the double EWA + est.enable_double_smoothing(); + + // exact same logic as above, just using single EWA as the source + let double_target = (0.9 * target + 0.09) / 0.99; + assert_eq!(est.steps_per_second(now), double_target); } #[test] fn test_estimator_rewind_position() { - let now = Instant::now(); + let mut now = Instant::now(); let mut est = Estimator::new(now); - est.record(0, now); + + now += Duration::from_secs(1); est.record(1, now); - assert_eq!(est.len(), 1); - // Should not panic. + + // should not panic + now += Duration::from_secs(1); est.record(0, now); - // Assert that the state of the estimator reset on rewind - assert_eq!(est.len(), 0); + // check that reset occurred (estimator at 1 event per sec) + now += Duration::from_secs(1); + est.record(1, now); + assert_eq!(est.steps_per_second(now), 1.0); + + // check that progress bar handles manual seeking let pb = ProgressBar::hidden(); pb.set_length(10); pb.set_position(1); @@ -695,6 +726,29 @@ mod tests { pb.set_position(0); } + #[test] + fn test_reset_eta() { + let mut now = Instant::now(); + let mut est = Estimator::new(now); + + // two per second, then reset + now += Duration::from_secs(1); + est.record(2, now); + est.reset(now); + + // now one per second, and verify + now += Duration::from_secs(1); + est.record(3, now); + assert_eq!(est.steps_per_second(now), 1.0); + } + + #[test] + fn test_duration_stuff() { + let duration = Duration::new(42, 100_000_000); + let secs = duration_to_secs(duration); + assert_eq!(secs_to_duration(secs), duration); + } + #[test] fn test_atomic_position_large_time_difference() { let atomic_position = AtomicPosition::new();