Opened 9 years ago

Closed 9 years ago

#12153 closed enhancement (fixed)

Make series of data available on sage_timeit return objects

Reported by: nbruin Owned by: jason
Priority: minor Milestone: sage-5.0
Component: misc Keywords:
Cc: Merged in: sage-5.0.beta0
Authors: William Stein Reviewers: Nils Bruin
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:


Since sage_timeit can already do repeats it may be useful to make all the gathered data available for analysis. See sage-devel

Attachments (2)

timeseries.patch (1.3 KB) - added by nbruin 9 years ago.
trac_12153.patch (2.6 KB) - added by was 9 years ago.
apply *only* this patch

Download all attachments as: .zip

Change History (11)

Changed 9 years ago by nbruin

comment:1 Changed 9 years ago by jason

In general, making a default argument [] is not a good idea, since only *one* list is created and is used as the default argument for all invocations of the function, meaning if that list is changed, all defaults for the function are changed.

comment:2 Changed 9 years ago by vbraun

I.e. change lists to tuples. If you don't want to have the array data modified then use a tuple.

comment:3 Changed 9 years ago by jason

Or I typically set the default to None and then test for None inside the function; if the value is None, then set it to a (new) empty list

Changed 9 years ago by was

apply *only* this patch

comment:4 Changed 9 years ago by was

  • Status changed from new to needs_review

comment:5 Changed 9 years ago by nbruin

Hm, seems that between 4.7.1 and 4.7.2 there have been some changes to sage_timeit. The patch doesn't apply for me. Just a few thoughts:

  • is "series" the right name? the compatibility with stats.TimeSeries seems to suggest so.
  • should "series" be scaled with the units s.stats[4] ?
  • don't use "mean" to illustrate statistics possibilities. "median" is almost certainly more meaningful since it doesn't depend on a metric, only on ordering. It is less sensitive to crazy outliers.

comment:6 Changed 9 years ago by was

Nils -- I'm using sage-4.8.alpha5.

  1. I'm good with series as the name
  1. I would like timings to always be in seconds (?), since otherwise it could be confusing to compare. Is that what happens now? Or are you proposing they *not* be in seconds?
  1. I just copied your example in which you used "mean"... but using something else might be useful.

There's a lot of good ideas on sage-devel. I could even see adding a "warm_up" option to timeit, where it runs something else first to warm things up and improve timing quality.

comment:7 Changed 9 years ago by nbruin

  • Status changed from needs_review to positive_review

Works for me on 4.6.alpha6 on sage.math. It can be made fancier of course, but the present patch already provides the basic functionality.

I don't think a warm_up option makes sense when s.series[3:] gives the same result (running "something else" would lead to different warming up characteristics in terms of memory use and cache filling. Running the same code multiple times before starting measurement is the only thing that has a good chance of leading to a measurement series where the first few aren't deviating. Whether that is what we want is of course a different discussion)

comment:8 Changed 9 years ago by jdemeyer

  • Authors set to William Stein
  • Milestone changed from sage-4.8 to sage-5.0
  • Reviewers set to Nils Bruin

comment:9 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.0.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.