#15030 closed defect (fixed)
Switch standard 2D plotting from `fast_float` to `fast_callable`
Reported by:  eviatarbach  Owned by:  

Priority:  major  Milestone:  sage6.2 
Component:  graphics  Keywords:  plot 
Cc:  Merged in:  
Authors:  Eviatar Bach  Reviewers:  Ralf Stephan 
Report Upstream:  N/A  Work issues:  compare patch with alternative by ppurka 
Branch:  7a9e07a (Commits)  Commit:  
Dependencies:  Stopgaps: 
Description
Switch the plot
command to use fast_callable
instead of fast_float
. This will make commands like plot(abs(log(x)), x)
work.
Attachments (1)
Change History (14)
Changed 5 years ago by
comment:1 Changed 5 years ago by
 Status changed from new to needs_review
comment:2 Changed 5 years ago by
Thanks for looking into this important issue!
Did you check to see what the performance implications of your patch are? I assume that fast_float
is faster than fast_callable
, but that is not based on any data.
comment:3 Changed 5 years ago by
Yes, fast_float
is slightly faster in my tests. However, I think the plan is to deprecate fast_float
anyway; see #5572.
comment:4 Changed 5 years ago by
This doesn't really fix #13559. Do you have an idea why? I thought this change to fast_callable
should take care of that other ticket too.
comment:5 Changed 5 years ago by
I think it's because plot
(look at the function generate_plot_points
in sage.plot.plot
) generates float points, and
sage: exp(1e5r) OverflowError: math range error
Maybe it should generate RealNumber? points instead (I guess they should also be faster than floats for many functions, because MPFR functions are functions are fast)? In any case, that doesn't affect this ticket.
comment:6 Changed 5 years ago by
Hmm.. so, you are changing only the plots for symbolic expressions. This patch makes the symbolic expression plots about five times slower.
sage: timeit('plot(abs(log(x)), (x, 20, 20))') # with patch 25 loops, best of 3: 18.5 ms per loop sage: timeit('plot(abs(log(x)), (x, 20, 20))') # without patch 125 loops, best of 3: 3.58 ms per loop
What do you think of the following change which introduces only a factor of two slowdown? The main problem seems to be that the fast float stuff does not handle complex numbers, and so we can explicitly specify the domain as CDF. I am not sure what is the default domain.
+ from sage.ext.fast_callable import fast_callable + from sage.rings.all import CDF + return fast_callable(self, vars=vars, expect_one_var=True, + domain=CDF)
The timeit results are as follows:
sage: timeit('plot(abs(log(x)), (x, 20, 20))') 125 loops, best of 3: 6 ms per loop
A few doctests in expression.pyx
fail, but they can be fixed (by changing their output).
comment:7 Changed 5 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:8 Changed 5 years ago by
 Status changed from needs_review to needs_work
 Work issues set to compare patch with alternative by ppurka
comment:9 Changed 5 years ago by
 Branch set to u/rws/switch_standard_2d_plotting_from__fast_float__to__fast_callable_
comment:10 Changed 5 years ago by
 Commit set to 7a9e07abeaf14171706e407173a342810031e200
 Keywords plot added
 Reviewers set to Ralf Stephan
 Status changed from needs_work to positive_review
this patch before sage: timeit('plot(abs(log(x)), (x, 20, 20))') 11.4 ms 2.4 sage: timeit('circle((1,1), 1) + plot(x^2, (x,0,5))') 2.7 ms 2.3 sage: timeit('plot(x^2,(x,300,500),ticks=[None,50000])') 10.8 ms 9.4 sage: i = CDF.0 # define i this way for maximum speed. sage: timeit('plot(lambda t: arg(zeta(0.5+t*i)), 1,27,rgbcolor=(0.8,0,0))') 52.2 ms 52.2 sage: timeit('plot([sin(n*x) for n in [1..4]], (0, pi))') 10.4 ms 10.2 sage: f = (2*x^3+2*x1)/((x2)*(x+1)) sage: timeit('plot([f, 2*x+2], 7,7, fill = {0: [1]})') 11.2 ms 11.1
so, in my opinion, there is only a slowdown where the original behaviour was incorrect. It is not correct to say "This patch makes the symbolic expression plots about five times slower."
I rebased the patch on 6.2.beta7. LGTM and tests fine in plot/
and symbolic/
.
New commits:
7a9e07a  Trac 15030: replace fast_float with fast_callable in 2D plotting

comment:11 Changed 5 years ago by
 Branch changed from u/rws/switch_standard_2d_plotting_from__fast_float__to__fast_callable_ to 7a9e07abeaf14171706e407173a342810031e200
 Resolution set to fixed
 Status changed from positive_review to closed
comment:12 followup: ↓ 13 Changed 5 years ago by
 Commit 7a9e07abeaf14171706e407173a342810031e200 deleted
Note that #13355 which is related, if not the same...
Also, anyone know how this impacts 3d plots?
comment:13 in reply to: ↑ 12 Changed 5 years ago by
Replying to kcrisman:
Also, anyone know how this impacts 3d plots?
I believe this has no impact on 3d plots. 3d plots from 2d objects goes via .plot3d()
methods which usually just take the already generated data points. 3d plots created directly are not affected by this patch.
The patch changes
plot
to usefast_callable
. It doesn't fixcontour_plot
orplot3d
, for example, since that seems to be more involved, and should be fixed in a future patch.Patchbot apply trac15030.patch