Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Provide smoothed diagnostics readings #4983

Closed
CAD97 opened this issue Jun 11, 2022 · 0 comments
Closed

Provide smoothed diagnostics readings #4983

CAD97 opened this issue Jun 11, 2022 · 0 comments
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Enhancement A new feature

Comments

@CAD97
Copy link
Contributor

CAD97 commented Jun 11, 2022

What problem does this solve or what need does it fill?

The current diagnostics collection offers two views into the data: getting the raw data, or a Simple Moving Average (the average of the last N data points). This is suboptimal, both because the duration of the last N samples depends on framerate, and because using SMA limits the visibility of a single frame spike (say, doing blocking IO on the main loop) to $1/N$, as it will be averaged with the surrounding frame's good measurements. (And as the SMA of frame time is used to calculate the raw FPS, the FPS will only show $1/N^2$ of the impact.)

What solution would you like?

The solution is instead of applying SMA, to use some form of Exponential Moving Average / Exponential Smoothing.

Tracking the EMA is not expensive; it can be done in a rolling fashion by updating it in place each measurement: $$EMA_t = \alpha x_t + (1 - \alpha) EMA_{t-1} = EMA_{t - 1} + \alpha (x_t - EMA_{t - 1})$$ Since we're dealing with measurements that come in at a varying frequency, we need a Time Adjusted EMA, as the normal EMA formula assumes a constant sample rate. For this we have $$\alpha = 1 - e^{-\Delta t / \tau}$$ where $\tau$ is the time (in the same units as $t$) that it takes to reflect $1 - \frac{1}{e} \approx 63.2\%$ of a change in signal. When $\Delta t \ll \tau$, $\alpha \approx \frac{\Delta t}{\tau}$, so we can skip the exponentiation — in fact, when $\Delta t \not\ll \tau$, the approximation results in reacting more to a change in source data, which in our case can be desirable!

Here are a few charts showing the different responses:

simple
spike-1
spike-2

(There are axis and titles, GitHub is just refusing to not crop them for some reason... right click open in new tab to see them.)

The overshoot in the third is because I deliberately did not clamp $\alpha$ to $[0,1]$ when generating these plots. Doing the clamp is likely desirable in the real implementation. I've used a SMA width of 20 samples (as that is what is currently used for frametime/framerate), and a $\tau$ of $2 / 21$ for EMA, as this supposedly approximates a the "weight" of a 20-bucket SMA.

Simulation notebook: https://gist.github.com/CAD97/ad71b3d626050702a304ce74032b6a0b

I've also already implemented the change in the engine

Bevy diff
diff --git a/crates/bevy_diagnostic/src/diagnostic.rs b/crates/bevy_diagnostic/src/diagnostic.rs
index ba8d1a0c0..03fcf208f 100644
--- a/crates/bevy_diagnostic/src/diagnostic.rs
+++ b/crates/bevy_diagnostic/src/diagnostic.rs
@@ -36,19 +36,31 @@ pub struct Diagnostic {
     pub suffix: Cow<'static, str>,
     history: VecDeque<DiagnosticMeasurement>,
     sum: f64,
+    ema: f64,
     max_history_length: usize,
 }
 
 impl Diagnostic {
     pub fn add_measurement(&mut self, value: f64) {
         let time = Instant::now();
+        if let Some(previous) = self.measurement() {
+            let delta = (time - previous.time).as_secs_f64();
+            // Smaller values respond faster to changes in the source data.
+            // Reflects 83% of a change in approximately this many seconds.
+            const SMOOTHNESS_FACTOR: f64 = 2.0 / 21.0;
+            let alpha = (delta / SMOOTHNESS_FACTOR).clamp(0.0, 1.0);
+            self.ema += alpha * (value - self.ema);
+        } else {
+            self.ema = value;
+        }
+
+        self.sum += value;
         if self.history.len() == self.max_history_length {
             if let Some(removed_diagnostic) = self.history.pop_front() {
                 self.sum -= removed_diagnostic.value;
             }
         }
 
-        self.sum += value;
         self.history
             .push_back(DiagnosticMeasurement { time, value });
     }
@@ -74,6 +86,7 @@ impl Diagnostic {
             history: VecDeque::with_capacity(max_history_length),
             max_history_length,
             sum: 0.0,
+            ema: 0.0,
         }
     }
 
@@ -104,6 +117,14 @@ impl Diagnostic {
         }
     }
 
+    pub fn smoothed(&self) -> Option<f64> {
+        if !self.history.is_empty() {
+            Some(self.ema)
+        } else {
+            None
+        }
+    }
+
     pub fn history_len(&self) -> usize {
         self.history.len()
     }
diff --git a/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs
index 2dd016e30..9140a4d56 100644
--- a/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs
+++ b/crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs
@@ -45,21 +45,6 @@ impl FrameTimeDiagnosticsPlugin {
         }
 
         diagnostics.add_measurement(Self::FRAME_TIME, time.delta_seconds_f64());
-        if let Some(fps) = diagnostics
-            .get(Self::FRAME_TIME)
-            .and_then(|frame_time_diagnostic| {
-                frame_time_diagnostic
-                    .average()
-                    .and_then(|frame_time_average| {
-                        if frame_time_average > 0.0 {
-                            Some(1.0 / frame_time_average)
-                        } else {
-                            None
-                        }
-                    })
-            })
-        {
-            diagnostics.add_measurement(Self::FPS, fps);
-        }
+        diagnostics.add_measurement(Self::FPS, 1.0 / time.delta_seconds_f64());
     }
 }
diff --git a/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs b/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs
index 728f1cd12..ea8f29886 100644
--- a/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs
+++ b/crates/bevy_diagnostic/src/log_diagnostics_plugin.rs
@@ -52,7 +52,7 @@ impl LogDiagnosticsPlugin {
     }
 
     fn log_diagnostic(diagnostic: &Diagnostic) {
-        if let Some(value) = diagnostic.value() {
+        if let Some(value) = diagnostic.smoothed() {
             if let Some(average) = diagnostic.average() {
                 info!(
                     target: "bevy diagnostic",
diff --git a/examples/stress_tests/bevymark.rs b/examples/stress_tests/bevymark.rs
index 92ead3b87..5d6692bd3 100644
--- a/examples/stress_tests/bevymark.rs
+++ b/examples/stress_tests/bevymark.rs
@@ -129,6 +129,38 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
                             color: Color::rgb(0.0, 1.0, 1.0),
                         },
                     },
+                    TextSection {
+                        value: "\nSmoothed FPS: ".to_string(),
+                        style: TextStyle {
+                            font: asset_server.load("fonts/FiraSans-Bold.ttf"),
+                            font_size: 40.0,
+                            color: Color::rgb(0.0, 1.0, 0.0),
+                        },
+                    },
+                    TextSection {
+                        value: "".to_string(),
+                        style: TextStyle {
+                            font: asset_server.load("fonts/FiraSans-Bold.ttf"),
+                            font_size: 40.0,
+                            color: Color::rgb(0.0, 1.0, 1.0),
+                        },
+                    },
+                    TextSection {
+                        value: "\nRaw FPS: ".to_string(),
+                        style: TextStyle {
+                            font: asset_server.load("fonts/FiraSans-Bold.ttf"),
+                            font_size: 40.0,
+                            color: Color::rgb(0.0, 1.0, 0.0),
+                        },
+                    },
+                    TextSection {
+                        value: "".to_string(),
+                        style: TextStyle {
+                            font: asset_server.load("fonts/FiraSans-Bold.ttf"),
+                            font_size: 40.0,
+                            color: Color::rgb(0.0, 1.0, 1.0),
+                        },
+                    },
                 ],
                 ..default()
             },
@@ -267,8 +299,14 @@ fn counter_system(
     }
 
     if let Some(fps) = diagnostics.get(FrameTimeDiagnosticsPlugin::FPS) {
-        if let Some(average) = fps.average() {
-            text.sections[3].value = format!("{:.2}", average);
+        if let Some(value) = fps.average() {
+            text.sections[3].value = format!("{:.2}", value);
+        }
+        if let Some(value) = fps.smoothed() {
+            text.sections[5].value = format!("{:.2}", value);
+        }
+        if let Some(measurement) = fps.measurement() {
+            text.sections[7].value = format!("{:.2}", measurement.value);
         }
     };
 }
diff --git a/examples/ui/text.rs b/examples/ui/text.rs
index 8e2c7fc71..9386d4b97 100644
--- a/examples/ui/text.rs
+++ b/examples/ui/text.rs
@@ -98,9 +98,9 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
 fn text_update_system(diagnostics: Res<Diagnostics>, mut query: Query<&mut Text, With<FpsText>>) {
     for mut text in query.iter_mut() {
         if let Some(fps) = diagnostics.get(FrameTimeDiagnosticsPlugin::FPS) {
-            if let Some(average) = fps.average() {
+            if let Some(value) = fps.smoothed() {
                 // Update the value of the second section
-                text.sections[1].value = format!("{:.2}", average);
+                text.sections[1].value = format!("{:.2}", value);
             }
         }
     }
diff --git a/examples/ui/text_debug.rs b/examples/ui/text_debug.rs
index 0f68b4707..f5799a10d 100644
--- a/examples/ui/text_debug.rs
+++ b/examples/ui/text_debug.rs
@@ -180,16 +180,16 @@ fn change_text_system(
     for mut text in query.iter_mut() {
         let mut fps = 0.0;
         if let Some(fps_diagnostic) = diagnostics.get(FrameTimeDiagnosticsPlugin::FPS) {
-            if let Some(fps_avg) = fps_diagnostic.average() {
-                fps = fps_avg;
+            if let Some(value) = fps_diagnostic.smoothed() {
+                fps = value;
             }
         }
 
         let mut frame_time = time.delta_seconds_f64();
         if let Some(frame_time_diagnostic) = diagnostics.get(FrameTimeDiagnosticsPlugin::FRAME_TIME)
         {
-            if let Some(frame_time_avg) = frame_time_diagnostic.average() {
-                frame_time = frame_time_avg;
+            if let Some(value) = frame_time_diagnostic.smoothed() {
+                frame_time = value;
             }
         }
 

so here's the impact on bevymark:

BevyMark.2022-06-10.23-29-42.mp4

Of course, the SMOOTHNESS_FACTOR I've included (here 2.0 / 21.0) should a) be tweaked to get a desirable default response curve, and b) ideally be exposed as a plugin parameter so it can be adjusted on a game-by-game (and perhaps diagnostic-by-diagnostic) basis.

What alternative(s) have you considered?

  • Do this in user code. Possible, but means running a system alongside the default diagnostics tracking system.
  • Just use the average. I think I've shown why this isn't desirable, as frame spikes are a bit too smoothed out when averaged with 19 other good frames.

Additional context

Some miscellaneous references I used when re-researching EMA for this issue:

https://en.wikipedia.org/wiki/Moving_average
https://en.wikipedia.org/wiki/Exponential_smoothing
https://stackoverflow.com/a/1027808
http://www.eckner.com/papers/Algorithms%20for%20Unevenly%20Spaced%20Time%20Series.pdf
Others I'm sure, that I closed the tab on and are lost in my browser history

User impact

Currently with the above patch, just a new function Diagnostic::smoothed(&self) -> Option<f64> that provides an exponentially smoothed reading of the diagnostic.

@CAD97 CAD97 added C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Jun 11, 2022
@Weibye Weibye added A-Diagnostics Logging, crash handling, error reporting and performance analysis and removed S-Needs-Triage This issue needs to be labelled labels Jun 11, 2022
@bors bors bot closed this as completed in c19aa59 Oct 24, 2022
james7132 pushed a commit to james7132/bevy that referenced this issue Oct 28, 2022
# Objective

- Add Time-Adjusted Rolling EMA-based smoothing to diagnostics.
- Closes bevyengine#4983; see that issue for more more information.

## Terms

- EMA - [Exponential Moving Average](https://en.wikipedia.org/wiki/Moving_average#Exponential_moving_average)
- SMA - [Simple Moving Average](https://en.wikipedia.org/wiki/Moving_average#Simple_moving_average)

## Solution

- We use a fairly standard approximation of a true EMA where $EMA_{\text{frame}} = EMA_{\text{previous}} + \alpha \left( x_{\text{frame}} - EMA_{\text{previous}} \right)$ where $\alpha = \Delta t / \tau$ and $\tau$ is an arbitrary smoothness factor. (See bevyengine#4983 for more discussion of the math.)
- The smoothness factor is here defaulted to $2 / 21$; this was chosen fairly arbitrarily as supposedly related to the existing 20-bucket SMA.
- The smoothness factor can be set on a per-diagnostic basis via `Diagnostic::with_smoothing_factor`.

---

## Changelog

### Added

- `Diagnostic::smoothed` - provides an exponentially smoothed view of a recorded diagnostic, to e.g. reduce jitter in frametime readings.

### Changed
- `LogDiagnosticsPlugin` now records the smoothed value rather than the raw value.
  - For diagnostics recorded less often than every 0.1 seconds, this change to defaults will have no visible effect.
  - For discrete diagnostics where this smoothing is not desirable, set a smoothing factor of 0 to disable smoothing.
  - The average of the recent history is still shown when available.
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this issue Dec 17, 2022
# Objective

- Add Time-Adjusted Rolling EMA-based smoothing to diagnostics.
- Closes bevyengine#4983; see that issue for more more information.

## Terms

- EMA - [Exponential Moving Average](https://en.wikipedia.org/wiki/Moving_average#Exponential_moving_average)
- SMA - [Simple Moving Average](https://en.wikipedia.org/wiki/Moving_average#Simple_moving_average)

## Solution

- We use a fairly standard approximation of a true EMA where $EMA_{\text{frame}} = EMA_{\text{previous}} + \alpha \left( x_{\text{frame}} - EMA_{\text{previous}} \right)$ where $\alpha = \Delta t / \tau$ and $\tau$ is an arbitrary smoothness factor. (See bevyengine#4983 for more discussion of the math.)
- The smoothness factor is here defaulted to $2 / 21$; this was chosen fairly arbitrarily as supposedly related to the existing 20-bucket SMA.
- The smoothness factor can be set on a per-diagnostic basis via `Diagnostic::with_smoothing_factor`.

---

## Changelog

### Added

- `Diagnostic::smoothed` - provides an exponentially smoothed view of a recorded diagnostic, to e.g. reduce jitter in frametime readings.

### Changed
- `LogDiagnosticsPlugin` now records the smoothed value rather than the raw value.
  - For diagnostics recorded less often than every 0.1 seconds, this change to defaults will have no visible effect.
  - For discrete diagnostics where this smoothing is not desirable, set a smoothing factor of 0 to disable smoothing.
  - The average of the recent history is still shown when available.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

- Add Time-Adjusted Rolling EMA-based smoothing to diagnostics.
- Closes bevyengine#4983; see that issue for more more information.

## Terms

- EMA - [Exponential Moving Average](https://en.wikipedia.org/wiki/Moving_average#Exponential_moving_average)
- SMA - [Simple Moving Average](https://en.wikipedia.org/wiki/Moving_average#Simple_moving_average)

## Solution

- We use a fairly standard approximation of a true EMA where $EMA_{\text{frame}} = EMA_{\text{previous}} + \alpha \left( x_{\text{frame}} - EMA_{\text{previous}} \right)$ where $\alpha = \Delta t / \tau$ and $\tau$ is an arbitrary smoothness factor. (See bevyengine#4983 for more discussion of the math.)
- The smoothness factor is here defaulted to $2 / 21$; this was chosen fairly arbitrarily as supposedly related to the existing 20-bucket SMA.
- The smoothness factor can be set on a per-diagnostic basis via `Diagnostic::with_smoothing_factor`.

---

## Changelog

### Added

- `Diagnostic::smoothed` - provides an exponentially smoothed view of a recorded diagnostic, to e.g. reduce jitter in frametime readings.

### Changed
- `LogDiagnosticsPlugin` now records the smoothed value rather than the raw value.
  - For diagnostics recorded less often than every 0.1 seconds, this change to defaults will have no visible effect.
  - For discrete diagnostics where this smoothing is not desirable, set a smoothing factor of 0 to disable smoothing.
  - The average of the recent history is still shown when available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Enhancement A new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants