Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#11885 closed defect (fixed)

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

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

Description (last modified by benjaminfjones)

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 benjaminfjones 6 years ago.
adds check that parent has attribute 'prec' before calling
trac_11885_v2.patch (1.0 KB) - added by benjaminfjones 6 years ago.
adds an exception check in call()

Download all attachments as: .zip

Change History (12)

Changed 6 years ago by benjaminfjones

adds check that parent has attribute 'prec' before calling

comment:1 Changed 6 years ago by leif

  • Authors set to Benjamin Jones
  • Description modified (diff)
  • Keywords precision added
  • Status changed from new to needs_review

comment:2 Changed 6 years ago by burcin

  • Reviewers set to Burcin Erocal
  • Status changed from needs_review to needs_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 6 years ago by benjaminfjones

adds an exception check in call()

comment:3 follow-up: Changed 6 years ago by benjaminfjones

  • Status changed from needs_work to needs_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 6 years ago by benjaminfjones

  • Description modified (diff)

comment:5 in reply to: ↑ 3 Changed 6 years ago by leif

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 6 years ago by burcin

  • Status changed from needs_review to positive_review

Thanks for the changes. Positive review.

comment:7 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-4.7.2 to sage-4.7.3

comment:8 Changed 6 years ago by jdemeyer

  • Merged in set to sage-4.7.3.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:9 Changed 6 years ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

comment:10 Changed 6 years ago by jdemeyer

  • Merged in changed from sage-4.7.3.alpha0 to sage-4.8.alpha0
  • Milestone set to sage-4.8
Note: See TracTickets for help on using tickets.