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

[Merged by Bors] - Add Exponential Moving Average into diagnostics #4992

Closed
wants to merge 1 commit into from

Conversation

CAD97
Copy link
Contributor

@CAD97 CAD97 commented Jun 11, 2022

Objective

Terms

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 Provide smoothed diagnostics readings #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.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Diagnostics Logging, crash handling, error reporting and performance analysis M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jun 12, 2022
@CAD97
Copy link
Contributor Author

CAD97 commented Jun 13, 2022

I should note that due to exactly how diagnostics were calculated/tracked, the translation is for numerically (almost1) equivalent now

before after
diagnostics[FRAME_TIME].value() diagnostics[FRAME_TIME].value()
diagnostics[FRAME_TIME].average() diagnostics[FRAME_TIME].average()
(none) diagnostics[FRAME_TIME].smoothed()
1 / diagnostics[FRAME_TIME].value() diagnostics[FPS].value()
diagnostics[FPS].value() (is also) diagnostics[FPS].average()
1 / diagnostics[FRAME_TIME].average() diagnostics[FPS].average()
diagnostics[FPS].average() (none)2
(none) diagnostics[FPS].smoothed()

Footnotes

  1. floats (aliens.png)

  2. If for some reason you do want the old FPS average3, this is achievable by averaging 20 frames worth of data of the new averaged FPS or FRAME_TIME. This is most simply done by recording a diagnostic with a 20-length history of 1 / diagnostics[FRAME_TIME].average() and taking its .average(); this is the old way that the FPS diagnostic was calculated.

  3. Perhaps for consistency of performance metrics over time? Though the engine update is likely to outweigh the impact of changing how the FPS diagnostic is recorded.

@CAD97
Copy link
Contributor Author

CAD97 commented Jun 13, 2022

I went ahead and added per-diagnostic customization of the smoothing factor so it can be more easily tuned.

I will reiterate that the default factor of $2 / 21$ here is completely arbitrary, and it should ideally be tuned for each application. $0.1$ is a rather common starting point, so we're in good company, but the exact smoothing feel desired differs per developer, and the default should probably be chosen by someone more familiar with bevy.

Discrete diagnostic measurements like asset or entity count probably will want to use a smoothing factor of $0$ to disable smoothing, especially as this PR currently prints the smoothed diagnostic in LogDiagnosticPlugin. This will avoid smoothing these counts and presenting a fractional number of entities 🙃 I haven't done so yet to let the stakeholders here decide appropriate tuning for the smoothing factor(s).

@hymm hymm self-requested a review June 21, 2022 19:50
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please. This is well implemented. I like that you changed the examples to use this, and I appreciate that you can control the smoothing on a per-diagnostic basis.

Copy link
Contributor

@hymm hymm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of having this built in and using this in the examples.

crates/bevy_diagnostic/src/diagnostic.rs Show resolved Hide resolved
@CAD97
Copy link
Contributor Author

CAD97 commented Oct 21, 2022

Rebased for the upstream changes. Note that #3871 already did the change from logging FPS as $1/SMA_\text{frametime}$ to just $1/\text{frametime}$, so this PR no longer contains that change.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 21, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 24, 2022
# Objective

- Add Time-Adjusted Rolling EMA-based smoothing to diagnostics.
- Closes #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 #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.
@bors bors bot changed the title Add Exponential Moving Average into diagnostics [Merged by Bors] - Add Exponential Moving Average into diagnostics Oct 24, 2022
@bors bors bot closed this Oct 24, 2022
@CAD97 CAD97 deleted the ema branch October 24, 2022 14:58
james7132 pushed a commit to james7132/bevy that referenced this pull request 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 pull request 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 pull request 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-Feature A new feature, making something new possible M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide smoothed diagnostics readings
3 participants