Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#11885 closed defect (fixed)

call function in sage.libs.mpmath.utils doesn't handle parent=parent(float)

Reported by: Benjamin Jones Owned by: Burcin Erocal
Priority: major Milestone: sage-4.8
Component: symbolics Keywords: mpmath call parent precision
Cc: Burcin Erocal Merged in: sage-4.8.alpha0
Authors: Benjamin Jones Reviewers: Burcin Erocal
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Benjamin Jones)

In multiple places in the Sage library the function sage.libs.mpmath.utils.call is called and inputs are passed along with their parents. If the parent of the input doesn't have a prec method, an AttributeError is raised:

sage: import sage.libs.mpmath.all as a
sage: z = float(4)
sage: a.call(a.e1, z)
0.00377935240984891
sage: a.call(a.e1, z, parent=parent(z))
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)

/Users/jonesbe/sage/sage-4.7.2.alpha2/devel/sage-test/sage/<ipython console> in <module>()

/Users/jonesbe/sage/latest/local/lib/python2.6/site-packages/sage/libs/mpmath/utils.so in sage.libs.mpmath.utils.call (sage/libs/mpmath/utils.c:5277)()

AttributeError: type object 'float' has no attribute 'prec'

This can be fixed with a simple check in in the call code in sage/libs/mpmath/utils.pyx.


Apply trac_11885_v2.patch to the Sage library.

Attachments (2)

trac_11885.patch (741 bytes) - added by Benjamin Jones 11 years ago.
adds check that parent has attribute 'prec' before calling
trac_11885_v2.patch (1.0 KB) - added by Benjamin Jones 11 years ago.
adds an exception check in call()

Download all attachments as: .zip

Change History (12)

Changed 11 years ago by Benjamin Jones

Attachment: trac_11885.patch added

adds check that parent has attribute 'prec' before calling

comment:1 Changed 11 years ago by Leif Leonhardy

Authors: Benjamin Jones
Description: modified (diff)
Keywords: precision added
Status: newneeds_review

comment:2 Changed 11 years ago by Burcin Erocal

Reviewers: Burcin Erocal
Status: needs_reviewneeds_work

Thanks for the patch. It looks good, though I have minor suggestions:

  • this needs to be doctested
  • using
    try:
        prec = parent.prec()
    except AttributeError:
        pass
    

might be faster than performing a test with hasattr() first, then looking up the prec() method again.

Changed 11 years ago by Benjamin Jones

Attachment: trac_11885_v2.patch added

adds an exception check in call()

comment:3 Changed 11 years ago by Benjamin Jones

Status: needs_workneeds_review

Good point! I made the change and added a doctest to call() to illustrate that the bug is fixed.

The change does give a speedup. The best case I found was running

sage: timeit('mputils.call(a.ei, 1.0r, parent=float)')

Before the old patch, I was getting about 87 micro seconds. With the new patch, the best time I got was about 60 micro seconds.

comment:4 Changed 11 years ago by Benjamin Jones

Description: modified (diff)

comment:5 in reply to:  3 Changed 11 years ago by Leif Leonhardy

Replying to benjaminfjones:

Good point! I made the change and added a doctest to call() to illustrate that the bug is fixed.

The change does give a speedup. The best case I found was running

sage: timeit('mputils.call(a.ei, 1.0r, parent=float)')

Before the old patch, I was getting about 87 micro seconds. With the new patch, the best time I got was about 60 micro seconds.

I'm not sure whether that's a deterministic result.

But Burcin is right, using try: ... is (at least) faster in case parent does have a prec() method.

comment:6 Changed 11 years ago by Burcin Erocal

Status: needs_reviewpositive_review

Thanks for the changes. Positive review.

comment:7 Changed 11 years ago by Jeroen Demeyer

Milestone: sage-4.7.2sage-4.7.3

comment:8 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-4.7.3.alpha0
Resolution: fixed
Status: positive_reviewclosed

comment:9 Changed 11 years ago by Jeroen Demeyer

Milestone: sage-4.7.3

Milestone sage-4.7.3 deleted

comment:10 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-4.7.3.alpha0sage-4.8.alpha0
Milestone: sage-4.8
Note: See TracTickets for help on using tickets.