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

Add warmup to EwmaRate #5443

Open
jhalterman opened this issue Jul 6, 2023 · 3 comments · May be fixed by #9514
Open

Add warmup to EwmaRate #5443

jhalterman opened this issue Jul 6, 2023 · 3 comments · May be fixed by #9514
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@jhalterman
Copy link
Member

Describe the bug

The current math.EwmaRate implementation does not require a warmup period, allowing a rate to be available after a single sample is added, which can lead to unexpected results, as described in #5394. Let's move warmup support from that PR into EwmaRate itself, to avoid problems with other users of this type.

@aknuds1 aknuds1 added the bug Something isn't working label Jul 7, 2023
@jhalterman jhalterman added the good first issue Good for newcomers label Feb 5, 2024
@samyakjain10
Copy link

Hi @jhalterman is the issue up for grab?

@jhalterman
Copy link
Member Author

Yes it is.

@slimjim777
Copy link

@jhalterman the implementation in the PR compares the firstUpdate time with the sliding window, whereas the VividCortex approach is to keep a count of "warmup samples" and to compare with the WARMUP_SAMPLES const. Were you thinking that math.EwmaRate should use the current approach or switch to the count approach?

The count approach looks like a cleaner approach for math.EwmaRate, otherwise there would need to be a way to pass in the first update and sliding window times.

@slimjim777 slimjim777 linked a pull request Oct 3, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants