Opened 14 years ago

Closed 14 years ago

#1938 closed defect (fixed)

[with bundle; enthusiastically positive review] Fast (double) function evaluation for plotting, etc.

Reported by: robertwb Owned by: robertwb
Priority: major Milestone: sage-2.10.1
Component: basic arithmetic Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

When one wishes to plot a 3d surface, one ends up evaluating one or more algebraic expressions several hundreds of times. This is especially slow if maxima is involved via the calculus package. Up until now the solution has been to plot lambda expressions, but this is neither intuitive, Sage-like, nor efficient (compared to operating on raw c doubles).

We need a way to get a quick-to-evaluate version of a symbolic expression (polynomial, ...) I would imagine this would be useful for optimization problems and simulations as well.

Attachments (4)

1938-fast-float-eval.hg (22.6 KB) - added by robertwb 14 years ago.
fast-float-merged.hg (42.3 KB) - added by robertwb 14 years ago.
1938-fast-float-fixes.hg (46.4 KB) - added by robertwb 14 years ago.
trac-1938-fix_some_docstrings.patch (4.0 KB) - added by was 14 years ago.
do apply this

Download all attachments as: .zip

Change History (20)

comment:1 Changed 14 years ago by robertwb

  • Milestone set to sage-2.10.2
  • Owner changed from somebody to robertwb
  • Status changed from new to assigned
  • Summary changed from Fast (double) function evaluation for plotting, etc. to [with bundle] Fast (double) function evaluation for plotting, etc.

Changed 14 years ago by robertwb

comment:2 Changed 14 years ago by robertwb

The attached bundle builds on and includes #1828 and #1862.

comment:3 Changed 14 years ago by mabshoff

Hi Robert,

the patch looks very nice, but I am seeing a massive number of merge conflict against 2.10.1.rc0. Could you take

http://sage.math.washington.edu/home/mabshoff/release-cycles-2.10.1/sage-2.10.1.rc0.tar

and rebase the patch? There have been a lot of tricky merges in the plotting code in 2.10.1.x, so it isn't really a surprise that there are problems :(

Cheers,

Michael

comment:4 follow-up: Changed 14 years ago by robertwb

Sure, I'll re-base it.

Is there a repo I could -upgrade from rather than re-building the whole thing from source?

comment:5 in reply to: ↑ 4 Changed 14 years ago by mabshoff

Replying to robertwb:

Sure, I'll re-base it.

Is there a repo I could -upgrade from rather than re-building the whole thing from source?

Nope, but it has been suggested a couple time to create a devel repo somewhere on sagemath.org to offer such a way to testbuild. I also saw that R failed to build for you on OSX 10.4 - I am building rc0 now on a 10.4 box to fix the issue.

Cheers,

Michael

comment:6 Changed 14 years ago by robertwb

Here's some timings:

sage: f(x,y,z) = sin(z)/(1+x^2+y^2)
sage: lambda_f = lambda x,y,z: math.sin(z)/(1+x^2+y^2)
sage: fast_f = f._fast_float_('x','y','z')

sage: sage: A = range(50000)
sage: sage: time for _ in A: r = f(1.0r, 2.0r, 3.0r)
CPU times: user 12.47 s, sys: 0.08 s, total: 12.54 s
Wall time: 12.73

sage: sage: time for _ in A: r = lambda_f(1.0r, 2.0r, 3.0r)
CPU times: user 1.32 s, sys: 0.01 s, total: 1.33 s
Wall time: 1.37

sage: sage: time for _ in A: r = fast_f(1.0r, 2.0r, 3.0r)
CPU times: user 0.04 s, sys: 0.00 s, total: 0.04 s
Wall time: 0.04

Changed 14 years ago by robertwb

comment:7 Changed 14 years ago by robertwb

The bundle just attached is merged with the plotting code of 2.10.1.rc0. Since William Stein was the other one who worked in this area, he should probably take a look at this as well to make sure I didn't regress anything.

comment:8 Changed 14 years ago by was

  • Summary changed from [with bundle] Fast (double) function evaluation for plotting, etc. to [with bundle; negative review] Fast (double) function evaluation for plotting, etc.

Wow, this is amazing compared to before!!!!!!!!!!!!!!!!! Wow! This is an awesome patch full of beautiful code. It will have a major impact on how the calculus code is used in the future, and make things like numerical integration, ode's, etc. way way way faster.

refereeing:

  • The the file morphism.pyx there are a bunch of print statements that shouldn't be there. They must have been for debugging purposes:
        cdef Element _call_c_impl(self, Element x):
            print type(self), self
            print <long>self._call_c
            print <long>Morphism._call_c
            print <long>FormalCoercionMorphism._call_c
            raise NotImplementedError
    
    
  • More doctests are needed, e.g., L3096 of calculus.py, has two functions without doctests:
        def fast_float_function(self, vars):
            return self._fast_float_(vars)
    
        def _fast_float_(self, *vars):
            try:
    
  • Is ext/ really the right place for fast_eval.pyx? Maybe. I'm just not sure...
  • I wish there were a few sentences at the top of fast_eval.pyx about what it does.
  • There's still a lot in fast_eval.pyx that needs doctests; it's coverage score is only 14%:
    teragon:ext was$ sage -coverage fast_eval.pyx
    ----------------------------------------------------------------------
    fast_eval.pyx
    ERROR: Please define a s == loads(dumps(s)) doctest.
    SCORE fast_eval.pyx: 14% (6 of 41)
    
  • This code in fast_eval.pyx is clearly TOTALLY WRONG and would give an infinite loop if it were actually tested:
    +    def sec(self):
    +        return ~self.sec()   
    

Otherwise it looks good to me. Fix the above then this patch should get marked with "positive review"

comment:9 Changed 14 years ago by robertwb

OK, I have fixed all the above issues, and added lots more documentation. As for placing it in sage/ext, I'm not sure but I can't think of another place that would be better.

The most recently attached bundle contains everything.

Changed 14 years ago by robertwb

comment:10 Changed 14 years ago by was

  • Summary changed from [with bundle; negative review] Fast (double) function evaluation for plotting, etc. to [with bundle; less negative review] Fast (double) function evaluation for plotting, etc.

There are still doctest failures in fast_eval.pyx. Otherwise this patch is good. I've posted a .patch that fixes most of those failures, plus a bunch of roughness in the paragraphs at the top. But there are three failing doctests left. Please fix and then this patch will be ready to go.

-- William

comment:11 Changed 14 years ago by robertwb

Do not apply the above patch last patch--everything passes on my computer including the lines

sage: list(h)
['push 1.5', 'load 0', 'add', 'call sin(1)']

instead of

sage: list(h)
['push 1.5', 'load 0', 'add', 'call 1 0x942acc20']

It looks like you didn't do a sage -b after applying merging the changes.

Changed 14 years ago by was

do apply this

comment:12 Changed 14 years ago by was

  • Milestone changed from sage-2.10.2 to sage-2.10.1

You're right -- I didn't "sage -br" after the merge. I redid the last patch, with a few fixes to typos and the writing in the paragraph, but without changing any doctests.

comment:13 Changed 14 years ago by was

  • Milestone changed from sage-2.10.1 to sage-2.10.2

comment:14 Changed 14 years ago by was

  • Summary changed from [with bundle; less negative review] Fast (double) function evaluation for plotting, etc. to [with bundle; enthusiastically positive review] Fast (double) function evaluation for plotting, etc.

This is now good to go, I think. Very nice.

comment:15 Changed 14 years ago by robertwb

Yes, the documentation fixes are from the new patch should be applied. Thanks.

comment:16 Changed 14 years ago by mabshoff

  • Milestone changed from sage-2.10.2 to sage-2.10.1
  • Resolution set to fixed
  • Status changed from assigned to closed

Merged 1938-fast-float-fixes.hg and trac-1938-fix_some_docstrings.patch in Sage 2.10.1.rc3

Note: See TracTickets for help on using tickets.