Ticket #3952 (closed defect: fixed)

Opened 3 months ago

Last modified 3 months ago

[with patch, positive review] make plot() and parametric_plot() use fast_float on their functions

Reported by: jason Assigned to: was
Priority: major Milestone: sage-3.1.2
Component: graphics Keywords:
Cc:

Description

Currently we have an error when calling these functions with constants. Using fast_float will take care of this and presumably make the functions *way* faster to boot.

Attachments

trac_3952.patch (13.6 kB) - added by mhansen on 08/26/2008 02:15:11 PM.

Change History

08/25/2008 02:58:21 PM changed by jason

See #2410 for an example of the error.

08/25/2008 07:01:11 PM changed by mhansen

This fixes #2409 and cuts the doctest time for plot.py in half.

08/25/2008 07:09:53 PM changed by mhansen

Note that this depends on #2410 and #3853.

08/25/2008 07:15:45 PM changed by mhansen

  • summary changed from make plot() and parametric_plot() use fast_float on their functions to [with patch, needs review] make plot() and parametric_plot() use fast_float on their functions.

08/25/2008 09:00:46 PM changed by cwitty

  • summary changed from [with patch, needs review] make plot() and parametric_plot() use fast_float on their functions to [with patch, positive review] make plot() and parametric_plot() use fast_float on their functions.

I've read through this patch, and it looks good to me. Positive review, assuming the doctests pass (I didn't run the doctests).

08/25/2008 09:19:44 PM changed by mabshoff

With the patch applied I am seeing one doctest failure: 

sage -t -long devel/sage/sage/calculus/desolvers.py
**********************************************************************
File "/scratch/mabshoff/release-cycle/sage-3.1.2.alpha1/tmp/desolvers.py", line 194:
    sage: P2 = parametric_plot((solnx,solny),0,1)
Exception raised:
    Traceback (most recent call last):
      File "/scratch/mabshoff/release-cycle/sage-3.1.2.alpha1/local/lib/python2.5/doctest.py", line 1228, in __run
        compileflags, 1) in test.globs
      File "<doctest __main__.example_4[17]>", line 1, in <module>
        P2 = parametric_plot((solnx,solny),Integer(0),Integer(1))###line 194:
    sage: P2 = parametric_plot((solnx,solny),0,1)
      File "/scratch/mabshoff/release-cycle/sage-3.1.2.alpha1/local/lib/python2.5/site-packages/sage/plot/plot.py", line 3735, in parametric_plot
        raise ValueError, "there must be exactly one free variable in funcs"
    ValueError: there must be exactly one free variable in funcs 
**********************************************************************

Cheers,

Michael

08/25/2008 11:48:47 PM changed by mabshoff

  • summary changed from [with patch, positive review] make plot() and parametric_plot() use fast_float on their functions to [with patch, needs work] make plot() and parametric_plot() use fast_float on their functions.

Until the doctest is fixed this needs some work :(

Cheers,

Michael

08/26/2008 02:15:11 PM changed by mhansen

  • attachment trac_3952.patch added.

08/26/2008 02:26:57 PM changed by mhansen

I've attached a new patch which fixes the issue.

08/26/2008 02:48:32 PM changed by mabshoff

  • summary changed from [with patch, needs work] make plot() and parametric_plot() use fast_float on their functions to [with patch, positive review] make plot() and parametric_plot() use fast_float on their functions.

Positive review on the patch. The affected doctest now passes and Mike did explain to me in IRC what was fixed: 

[2:41pm] mabshoff: mhansen: #3592 - what is changed?
[2:41pm] mabshoff: Did you fix the doctest or did you do more?
[2:42pm] mhansen: Oh, I changed a "!= 1" to a " > 1" in the new code (which is what is should have been).
[2:42pm] mabshoff: ok

Cheers,

Michael

08/26/2008 03:16:42 PM changed by mabshoff

  • status changed from new to closed.
  • resolution set to fixed.

Merged in Sage 3.1.2.alpha1