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

Revised/additional summary page #437

Merged
merged 16 commits into from Jul 19, 2016
Merged

Revised/additional summary page #437

merged 16 commits into from Jul 19, 2016

Conversation

pv
Copy link
Collaborator

@pv pv commented Jun 15, 2016

Add a second summary page, showing a list of benchmarks (instead of a grid).

The list shows benchmarks only for one choice of environment parameters (machine, cpu, packages, ...) at once.

Also show how the benchmark performance has evolved "recently", some ideas borrowed from http://speed.pypy.org/changes/

Some overlap with the Regressions display, with the exception that regressions shows regressions across all branches, and does not show improved results.

Demo: https://pv.github.io/numpy-bench/#summarylist
https://pv.github.io/scipy-bench/#summarylist

  • what information should it actually show?
  • what does "recently" mean? 1 month, 10 revisions, ...?
  • what to do with benchmarks with no results?
  • the param selector should deal sensibly if there are no results for some param combination
  • tests, etc. polish
  • a selector for the time/revision range that you want to consider (e.g. show the worst regression ever vs. the last one vs. benchmark-specific configuration in the config file)
  • maybe add expandable view to show all regressions?
  • the popup graphs

@philpep
Copy link
Contributor

philpep commented Jun 16, 2016

This looks awesome, thanks !!

About "what does "recently" mean? 1 month, 10 revisions, ...?"

Looking the latest step (regression or improvement) could be enough. It seems this is what you're currently doing with last_piece = steps[-1].

Now we have this "improvement" information, we could provide a "per commit" view that show all regressions and improvement where a particular commit is involved in. This will help for our use case (mercurial revsets) where a single change can impact multiple benchmarks in a good or bad manner, in this case we may want to promote improvement on very used revsets and accept some regression on rarely used ones.

But indeed this new page overlap with the regression page. Maybe we could merge them in a single page and add some selectors to filter showed data.

I really like the colors and the percentage here. What represent the data after the ±, is it the noise ?

@pv
Copy link
Collaborator Author

pv commented Jun 16, 2016

Yes, I think it is relatively straightforward to add different views to the data, and the per-commit one could also be useful. The difficult part is just deciding what to present and how. (I'm a bit time-constrained currently, so don't let this PR/WIP block if you're thinking of something, since this may take long to finish.)

I don't see the overlap with the regression pageas necessarily as a problem in itself, but it can be useful to import the ideas that can be imported. Potentially for the present page: (added to top)

The +- shows the noise level of the value (= average deviation from the median in the last piecewise step). It maybe should be multiplied by 2 to be more intuitive... Added as inspired by speed.pypy.org.

@pv pv force-pushed the summarylist branch 2 times, most recently from b4919d9 to d03e8d7 Compare June 23, 2016 20:31
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 71.833% when pulling 7d020df on pv:summarylist into b2eb264 on spacetelescope:master.

@coveralls
Copy link

coveralls commented Jul 6, 2016

Coverage Status

Coverage decreased (-13.9%) to 57.545% when pulling b64c7a5 on pv:summarylist into b2eb264 on spacetelescope:master.

@coveralls
Copy link

coveralls commented Jul 7, 2016

Coverage Status

Coverage decreased (-13.9%) to 57.512% when pulling 9c4dd94 on pv:summarylist into b2eb264 on spacetelescope:master.

@coveralls
Copy link

coveralls commented Jul 7, 2016

Coverage Status

Coverage decreased (-13.9%) to 57.512% when pulling 9c4dd94 on pv:summarylist into b2eb264 on spacetelescope:master.

@pv pv force-pushed the summarylist branch 2 times, most recently from 983f8d0 to 3343255 Compare July 8, 2016 18:00
@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 57.558% when pulling 3343255 on pv:summarylist into cfe55f2 on spacetelescope:master.

@coveralls
Copy link

coveralls commented Jul 8, 2016

Coverage Status

Coverage increased (+0.6%) to 57.526% when pulling 92d4fb4 on pv:summarylist into cfe55f2 on spacetelescope:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 57.558% when pulling a414753 on pv:summarylist into cfe55f2 on spacetelescope:master.

@pv pv changed the title WIP: revised/additional summary page Revised/additional summary page Jul 9, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 57.558% when pulling 729e63c on pv:summarylist into cfe55f2 on spacetelescope:master.

@pv
Copy link
Collaborator Author

pv commented Jul 10, 2016

I think this is done now. Additions can be added later.

pv added 8 commits July 16, 2016 18:21
Move navigation links to the top bar. Make regressions page appear full
width.
If it's too small, the bootstrap top bar overlaps the regression list
and the popup plot doesn't appear.
The cached test data is cleared on asv version number changes,
or manually via py.test --cache-clear
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 57.591% when pulling 1b293e0 on pv:summarylist into b1cc0a9 on spacetelescope:master.

@coveralls
Copy link

coveralls commented Jul 19, 2016

Coverage Status

Coverage increased (+0.5%) to 57.591% when pulling ae8c02a on pv:summarylist into b1cc0a9 on spacetelescope:master.

@pv pv merged commit fbd9cc8 into airspeed-velocity:master Jul 19, 2016
@pv
Copy link
Collaborator Author

pv commented Jul 19, 2016

Merging --- after dogfooding for a while, seems to work ok.

@pv pv deleted the summarylist branch August 19, 2018 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants