-
Notifications
You must be signed in to change notification settings - Fork 149
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
Handle Noise Better #1489
Handle Noise Better #1489
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Displaying how noisy a certain benchmark is sounds like a great idea.
I'm not sure about the sorting though. Maybe I'll just need to get used to it. Maybe we could add a checkbox to toggle between sorting by significance/absolute change?
It's probably a bit unintuitive that the tables contain max/min absolute change, but that won't be reflected in the table order.
|
I think sorting by the change itself still makes sense, rather than the significance factor. Two benchmarks with roughly equivalent significance (e.g., 10x - basically "high") should be ordered by the change each had, which with different noise levels etc might well be different. |
|
Thanks for this PR, it's good that this will be improving. Comments:
|
209d415
to
73ea936
Compare
|
I've reverted the change to sorting, and I've changed the ordering of the columns to put the threshold first and then the factor. |
This adjusts historical analysis to handle noise more quickly. The following changes are included: