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.

Sign up for GitHub

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

Jump to bottom

testing: investigate possible bias due to time estimation #27168

Closed
josharian opened this issue Aug 23, 2018 · 3 comments
Closed

testing: investigate possible bias due to time estimation #27168

josharian opened this issue Aug 23, 2018 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.13

Comments

@josharian
Copy link
Contributor

In #24735 (comment) I wrote:

Suppose we have a benchmark with high variance. We use our estimates to try to get near the benchtime. We are more likely to exceed the benchtime if we get a particularly slow run. A particularly fast run is more likely to trigger another benchmark run.
The current approach thus introduces bias.
One simple way to fix this would be to decide when our estimate is going to be "close enough", that is, when we are one iteration way from being done, and then stick with that final iteration even if it falls short of the benchtime.

This issue is to follow up on this concern, independently of #24735, which is really about something else.
@josharian josharian added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 23, 2018
@josharian josharian added this to the Go1.12 milestone Aug 23, 2018
@josharian josharian mentioned this issue Aug 23, 2018
Closed
@rsc
Copy link
Contributor

rsc commented Sep 26, 2018

I don't see a decision here. Moving to NeedsInvestigation (probably by Josh).

@rsc rsc added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 26, 2018
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 26, 2018
@josharian
Copy link
Contributor Author

Still needs investigation. I think the theoretical concern is real; the real question is whether it matters in practice.

While I'm thinking of it, another simple fix for this would be to accept benchmark execution times that are with a range of the goal (say +/- 10%) rather than requiring that they be strictly >= the goal. That'd also reduce pressure to overestimate b.N (see also CL 112155 and #27217) and make benchmarks run faster overall.

@josharian josharian mentioned this issue Jan 13, 2019
Open
@josharian josharian mentioned this issue Jan 23, 2019
Open
@andybons andybons modified the milestones: Go1.12, Go1.13 Feb 12, 2019
@josharian
Copy link
Contributor Author

I have failed to create a benchmark that demonstrates this bias. I'm going to call this theoretical only.

@golang golang locked and limited conversation to collaborators Mar 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
5 participants
@josharian @rsc @andybons @gopherbot and others