Opened 10 years ago

Closed 6 years ago

Last modified 4 years ago

#5814 closed enhancement (fixed)

%prun doesn't work in the notebook

Reported by: rlm Owned by: tkluck
Priority: major Milestone: sage-5.9
Component: notebook Keywords:
Cc: Merged in: sage-5.9.beta4
Authors: Timo Kluck Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by tscrim)

In ipython, one can use the %prun directive to use a profiler on cell contents. We can support the same in sage. The attached patches allow this.

Apply:

Attachments (4)

trac_5814_enable_prun_in_notebook.patch (2.5 KB) - added by tkluck 6 years ago.
trac_5814_add_doctests_to_prun_object.patch (1.6 KB) - added by tkluck 6 years ago.
trac_5814-prun_notebook-review-ts.patch (2.0 KB) - added by tscrim 6 years ago.
trac_5814-prun_notebook-combined.patch (3.8 KB) - added by tscrim 6 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 10 years ago by was

  • Type changed from defect to enhancement

I'm changing this to an enhancement. It's not a bug that "random" ipython features don't work in the notebook. There's no reason they should until they get implemented.

The prun functionality -- which is really the python profiler -- could be used in the notebook by people who know enough about how the profiler works under the hood. %prun is just IPython's wrapper around Python's standard profiler to make it easier to use.

comment:2 Changed 7 years ago by tkluck

  • Report Upstream set to N/A
  • Status changed from new to needs_review

Here's a branch that implements this feature. Feedback appreciated!

https://github.com/tkluck/sagenb/commits/support_prun

comment:3 Changed 7 years ago by kini

  • Milestone changed from sage-5.3 to sage-duplicate/invalid/wontfix
  • Status changed from needs_review to positive_review

Thanks, tkluck. I see you made a pull request at https://github.com/sagemath/sagenb/pull/91 . Please continue any review process there :)

I'm marking this ticket as invalid.

comment:4 Changed 7 years ago by jdemeyer

  • Resolution set to invalid
  • Reviewers set to Keshav Kini
  • Status changed from positive_review to closed

Changed 6 years ago by tkluck

comment:5 Changed 6 years ago by tkluck

  • Authors set to Timo Kluck
  • Milestone changed from sage-duplicate/invalid/wontfix to sage-feature
  • Owner changed from boothby to tkluck

It turns out that this does need to be tackled in the sage library itself. It is possible to add a prun object there, which will be automatically used without having to do any special handling in the notebook. I've closed my pull request for the notebook and am reopening this ticket.

comment:6 Changed 6 years ago by jdemeyer

  • Resolution invalid deleted
  • Status changed from closed to new

comment:7 Changed 6 years ago by tkluck

  • Status changed from new to needs_review

comment:8 Changed 6 years ago by tscrim

Just wanting to make sure that this patch ready for review, there are a few people here at Sage Days 45 that would like this feature. (Also do you know someone who would have the expertise to do a full review?)

My quick comment is that you'll need to give doctests to your functions.

Thanks,
Travis

Last edited 6 years ago by tscrim (previous) (diff)

Changed 6 years ago by tkluck

comment:9 Changed 6 years ago by tkluck

  • Description modified (diff)

comment:10 Changed 6 years ago by tkluck

Thanks for your comment, Travis. I just added the doctests. I'm not sure whether any special expertise is necessary, so please go ahead and give this a try.

Changed 6 years ago by tscrim

comment:11 Changed 6 years ago by tscrim

  • Description modified (diff)
  • Reviewers changed from Keshav Kini to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Hey Timo,

I've uploaded a small review patch which fixes the doctests and gives full coverage. Thank you for the patch.

Best,
Travis

sage -t ../misc/prun.py
    [6 tests, 0.0 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.2 seconds
    cpu time: 0.0 seconds
    cumulative wall time: 0.0 seconds

comment:12 follow-up: Changed 6 years ago by nbruin

It's probably outside the scope of this ticket, but the really useful thing would be if %prun ... in the notebook would return an interactive browser for the Stat object that is returned by cProfile().runctx(...). While a basic printout of profiling data gives some indication, one usually needs to dig around a little in the data to find what the real bottleneck is (see where most calls come from, switch between cumulative and proper time, etc.)

Writing such a browser might be a nice exercise for a student who is interested in interface programming. I bet there are good profiler browsers around to get inspiration from.

A tool I haven't used yet but looks like a decent effort is runsnakerun. Duplicating that effort for the sage notebook might not seem like such a smart idea. Perhaps we can make the tool more easily accessible from sage? Given that it uses wxPython that might not be so very easily done ... I ended up installing it using OS tools, meaning that I have to write the profile data to a file and analyse it separately.

Last edited 6 years ago by nbruin (previous) (diff)

comment:13 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-feature to sage-5.9

comment:14 in reply to: ↑ 12 Changed 6 years ago by tscrim

Replying to nbruin:

It's probably outside the scope of this ticket, but the really useful thing would be if %prun ... in the notebook would return an interactive browser for the Stat object that is returned by cProfile().runctx(...). While a basic printout of profiling data gives some indication, one usually needs to dig around a little in the data to find what the real bottleneck is (see where most calls come from, switch between cumulative and proper time, etc.)

+1/2 since being able to read/interpret the raw data is a good skill to have IMO. However a graphical (or at least a tree structure) of the data is also very useful.

Writing such a browser might be a nice exercise for a student who is interested in interface programming. I bet there are good profiler browsers around to get inspiration from.

+1 here. I feel like we need a wiki page to keep a list of "good student projects"...

A tool I haven't used yet but looks like a decent effort is runsnakerun. Duplicating that effort for the sage notebook might not seem like such a smart idea. Perhaps we can make the tool more easily accessible from sage? Given that it uses wxPython that might not be so very easily done ... I ended up installing it using OS tools, meaning that I have to write the profile data to a file and analyse it separately.

I've seen runsnakerun used in a few places (although I haven't used it myself), and instead of duplicating it in the notebook, perhaps we could link it into sage (notebook) as some type of package? Actually...perhaps what we should do is have a variable/function which calls your favorite profiler on the executed command(s), something like:

%profile
2 + 2

comment:15 Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_work

The first patch isn't a proper HG changeset (patches should be generated using hg export, the second patch is fine).

Changed 6 years ago by tscrim

comment:16 Changed 6 years ago by tscrim

  • Description modified (diff)
  • Status changed from needs_work to positive_review

Fixed by combining all the patches into one. I hope that's okay.

comment:17 Changed 6 years ago by tkluck

Thanks for your work on this, Travis! Your changes look excellent.

I'm kind of surprised that you found 20 function calls where I had 3. I'll try that again later, but I trust you on it.

comment:18 Changed 6 years ago by tscrim

That also seemed to be what the patchbot was getting, but if you're still getting fewer function calls, then we will have to change the test slightly. I'm also running these from the command line where I know the preparser is replacing 1 with Integer(1) which is an extra function call. I'm guessing you were doing the testing from the notebook? I'm 99% certain the notebook is doing the same, but it might evaluating these before it actually executes the %prun test...I'll do some tests later today too.

For patchbot:

Apply: trac_5814-prun_notebook-combined.patch

comment:19 Changed 6 years ago by tscrim

In the notebook I get 3 function calls

         3 function calls in 0.007 seconds

   Ordered by: standard name

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.007    0.007    0.007    0.007 <string>:1(<module>)
        1    0.000    0.000    0.000    0.000 {len}
        1    0.000    0.000    0.000    0.000 {method 'disable' of '_lsprof.Profiler' objects}

with the command line:

         3 function calls in 0.000 seconds

   Ordered by: standard name

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.000    0.000    0.000    0.000 <string>:1(<module>)
        1    0.000    0.000    0.000    0.000 {len}
        1    0.000    0.000    0.000    0.000 {method 'disable' of '_lsprof.Profiler' objects}

So it's something specific to the doctesting framework that's not reflected in sage while it's directly running (I've noticed this in a few other places before too).

comment:20 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.9.beta4
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:21 Changed 4 years ago by kcrisman

Does this actually work in the notebook? (Sometimes Ipython upgrades cause trouble with such magic.) I just get

NameError: name 'prun' is not defined

in 6.3 and 6.4.beta6.

comment:22 Changed 4 years ago by tscrim

I'm almost sure it did...but it's been so long that I can be certain. Considering we were using iPython 0.13 or something like that at the time, I wouldn't be surprised if something broke along the way.

comment:23 Changed 4 years ago by kcrisman

Does it work for you now?

comment:24 Changed 4 years ago by tscrim

Nope.

Note: See TracTickets for help on using tickets.