Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#6985 closed defect (fixed)

[with patch, positive review] complex_plot needs to use fast_callable

Reported by: jason Owned by: mhansen
Priority: major Milestone: sage-4.1.2
Component: graphics Keywords:
Cc: kcrisman Merged in: Sage 4.1.2.alpha4
Authors: Mike Hansen, Jason Grout Reviewers: Jason Grout, Karl-Dieter Crisman
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Timing differences:

{{{
sage: f(x) = x^2                   
sage: %time P = complex_plot(f, (-10, 10), (-10, 10))
CPU times: user 1.99 s, sys: 0.00 s, total: 2.00 s
Wall time: 2.02 s
sage: g = fast_callable(f, domain=CC, vars='x')
sage: %time Q = complex_plot(g, (-10, 10), (-10, 10))
CPU times: user 0.54 s, sys: 0.01 s, total: 0.55 s
Wall time: 0.57 s
sage: h = fast_callable(f, domain=CDF, vars='x')
sage: %time R = complex_plot(h, (-10, 10), (-10, 10))
CPU times: user 0.20 s, sys: 0.00 s, total: 0.20 s
Wall time: 0.21 s
}}}

Attachments (3)

trac_6985.patch (1.4 KB) - added by mhansen 10 years ago.
trac-6985-CDF-domain.patch (1.4 KB) - added by jason 10 years ago.
apply on top of previous patch
trac_6985-CDF_and_reviewer.patch (1.3 KB) - added by kcrisman 10 years ago.
Apply on top of first patch, instead of other CDF patch

Download all attachments as: .zip

Change History (25)

comment:1 Changed 10 years ago by jason

  • Cc kcrisman added

Changed 10 years ago by mhansen

comment:3 follow-up: Changed 10 years ago by mhansen

  • Authors set to Mike Hansen
  • Owner changed from was to mhansen
  • Status changed from new to assigned
  • Summary changed from complex_plot needs to use fast_callable to [with patch, needs review] complex_plot needs to use fast_callable

It fixes those issues, but not directly.

In setup_for_eval_on_grid, we should check for not just types.FunctionType?, but instead for (types.FunctionType?, types.LambdaType?, types.BuiltinFunctionType?, types.BuiltinMethodType?). Also, everything should be converted from fast_float to fast_callable. This would be a separate ticket.

comment:4 in reply to: ↑ 3 Changed 10 years ago by jason

Replying to mhansen:

It fixes those issues, but not directly.

In setup_for_eval_on_grid, we should check for not just types.FunctionType?, but instead for (types.FunctionType?, types.LambdaType?, types.BuiltinFunctionType?, types.BuiltinMethodType?). Also, everything should be converted from fast_float to fast_callable. This would be a separate ticket.

Can you make another ticket for this?

comment:5 Changed 10 years ago by jason

  • Summary changed from [with patch, needs review] complex_plot needs to use fast_callable to [with patch, needs work] complex_plot needs to use fast_callable

There seems to be a regression: %time complex_plot(exp(x)-sin(x), (-10, 10), (-10, 10)) takes 21 seconds before the patch, but 28 seconds after the patch for me.

Note:

sage: f(x)=exp(x)-sin(x)
sage: fcomplex=fast_callable(f, domain=complex, expect_one_var=True)sage: fCDF=fast_callable(f, domain=CDF, expect_one_var=True)
sage: %timeit f(4j)
100 loops, best of 3: 2.21 ms per loop
sage: %timeit fcomplex(4j)
100 loops, best of 3: 2.7 ms per loop
sage: %timeit fCDF(4j)
100000 loops, best of 3: 7.94 µs per loop

So maybe the fast_callable in the patch should use domain CDF!

(this seems really odd to me, but I can't argue with the timings above!)

comment:6 Changed 10 years ago by jason

In fact, we see the same sort of speedup just with exp(x):

sage: f(x)=exp(x)
sage: fcomplex=fast_callable(f, domain=complex, expect_one_var=True)
sage: fCDF=fast_callable(f, domain=CDF, expect_one_var=True)
sage: %timeit f(4j)
100 loops, best of 3: 2.12 ms per loop
sage: %timeit fcomplex(4j)
100 loops, best of 3: 2.39 ms per loop
sage: %timeit fCDF(4j)
100000 loops, best of 3: 7.32 µs per loop

comment:7 Changed 10 years ago by jason

The fast_callable documentation mentions a special interpreter for float, which is the same as for RDF, and also a special interpreter for CDF. It never mentions complex. So maybe that's a bug/feature request for fast_callable...

comment:8 Changed 10 years ago by jason

  • Summary changed from [with patch, needs work] complex_plot needs to use fast_callable to [with patch, needs review] complex_plot needs to use fast_callable

My simple patch to make the domain CDF should also maybe be reviewed. All of the plots are even faster now than with domain=complex.

comment:9 Changed 10 years ago by jason

For the tour:

BEFORE PATCH:

sage: %time complex_plot(exp(x)-sin(x), (-20, 20), (-20, 20))
/home/jason/.sage/temp/littleone/4745/_home_jason__sage_init_sage_0.py:1: DeprecationWarning: Substitution using function-call syntax and unnamed arguments is deprecated and will be removed from a future release of Sage; you can use named arguments instead, like EXPR(x=..., y=...)
  # -*- coding: utf-8 -*-
CPU times: user 20.51 s, sys: 0.40 s, total: 20.91 s
Wall time: 21.09 s

AFTER PATCH:

sage: %time complex_plot(exp(x)-sin(x), (-20, 20), (-20, 20))
CPU times: user 0.03 s, sys: 0.01 s, total: 0.04 s
Wall time: 0.05 s

comment:10 Changed 10 years ago by jason

(oh, and positive review to mhansen's patch. My patch should still be reviewed.)

comment:11 Changed 10 years ago by kcrisman

These don't apply for me properly - I don't have the stuff after the Riemann Zeta function and options, but before the actual code, e.g.

sage: P = complex_plot(f, (-10, 10), (-10, 10)) 
sage: Q = complex_plot(g, (-10, 10), (-10, 10)) 
sage: R = complex_plot(h, (-10, 10), (-10, 10)) 

Is that in some other patch, or was it removed before 4.1.2.alpha2?

comment:12 Changed 10 years ago by kcrisman

  • Reviewers set to Jason Grout, Karl-Dieter Crisman
  • Summary changed from [with patch, needs review] complex_plot needs to use fast_callable to [with patch, needs work] complex_plot needs to use fast_callable

Oh, and I think there should still be at least one example of the type

sage: complex_plot(sqrt, (-5, 5), (-5, 5))

to show that it is still possible.

comment:13 Changed 10 years ago by jason

Oh, this patch depends on #6947.

comment:14 Changed 10 years ago by jason

Some work needs to be done on fast_callable to make it be able to replace fast_float: see #5572.

comment:15 Changed 10 years ago by kcrisman

Does this mean this patch is not ready for review?

I was going to try to review it (after applying #6947) later on... unfortunately, my main computer is on the fritz so it's back to 1 GB of memory and recompiling 4.1.2.alpha2 from scratch.

comment:16 Changed 10 years ago by jason

Nope, this patch is ready to go in (maybe after you add a patch with the doctest you like :).

Changed 10 years ago by jason

apply on top of previous patch

comment:17 Changed 10 years ago by jason

  • Summary changed from [with patch, needs work] complex_plot needs to use fast_callable to [with patch, needs review] complex_plot needs to use fast_callable

I just updated the patch to have your example there too.

comment:18 Changed 10 years ago by jason

  • Authors changed from Mike Hansen to Mike Hansen, Jason Grout

Changed 10 years ago by kcrisman

Apply on top of first patch, instead of other CDF patch

comment:19 Changed 10 years ago by kcrisman

  • Summary changed from [with patch, needs review] complex_plot needs to use fast_callable to [with patch, positive review] complex_plot needs to use fast_callable

Okay, positive review. I put the example I wanted under tests instead, because it's really noticeably slower.

comment:20 Changed 10 years ago by kcrisman

Of course, this will unfortunately necessitate a one-line change in the patch for #7008 so it applies properly.

comment:21 Changed 10 years ago by mvngu

  • Merged in set to Sage 4.1.2.alpha3
  • Resolution set to fixed
  • Status changed from assigned to closed

Merged patches in this order:

  1. trac_6985.patch
  2. trac_6985-CDF_and_reviewer.patch

comment:22 Changed 10 years ago by mvngu

  • Merged in changed from Sage 4.1.2.alpha3 to Sage 4.1.2.alpha4

There is no 4.1.2.alpha3. Sage 4.1.2.alpha3 was William Stein's release for working on making the notebook a standalone package.

Note: See TracTickets for help on using tickets.