Opened 4 years ago

Closed 3 years ago

#14984 closed defect (fixed)

Cannot plot functions that use mpmath if complex numbers occur in the image

Reported by: eviatarbach Owned by:
Priority: major Milestone: sage-6.2
Component: numerical Keywords:
Cc: kcrisman Merged in:
Authors: Eviatar Bach Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: bdcc06d (Commits) Commit: bdcc06d6025a4c250ac1110c279a3ad14c20c7c8
Dependencies: Stopgaps:

Description

Currently, trying to plot a function which uses mpmath and returns a complex number in the plotting domain will return an error. For example,

sage: from sage.libs.mpmath import utils
sage: import mpmath
sage: class Test(BuiltinFunction):
....:     def __init__(self):
....:         BuiltinFunction.__init__(self, name='test', nargs=1)
....:     def _evalf_(self, x, parent):
....:         return utils.call(mpmath.log, x, parent=parent)
....:     
sage: test = Test()
sage: plot(test, x, -1, 1)
AttributeError: type object 'float' has no attribute 'complex_field'

Attachments (2)

trac14984.patch (1.0 KB) - added by eviatarbach 4 years ago.
trac14984_2.patch (2.0 KB) - added by eviatarbach 4 years ago.

Download all attachments as: .zip

Change History (11)

Changed 4 years ago by eviatarbach

comment:1 Changed 4 years ago by eviatarbach

  • Component changed from coercion to numerical
  • Status changed from new to needs_review

The patch fixes the problem by making call in libs.mpmath.utils return the complex type if a complex number is returned from the mpmath call and float is the parent. This is consistent with the behaviour for Sage types, as the documentation explains: "the result will be coerced to P (or the corresponding complex field if necessary)".

Patchbot apply trac14984.patch

Last edited 4 years ago by eviatarbach (previous) (diff)

comment:2 Changed 4 years ago by kcrisman

  • Cc kcrisman added

comment:3 Changed 4 years ago by ppurka

What are the possible inputs of the parent keyword? For example, it will still crash if parent=int or parent=Integer. I think it should be modified to return complex(y) only after either doing something like this

    try:
        return parent(y)
    except TypeError:
        if hasattr(parent, 'complex_field'):
            return parent.complex_field()(y)
        else:
            return complex(y)

or, by doing something like this:

    try:
        return parent(y)
    except TypeError:
        try:
            return parent.complex_field()(y)
        except AttributeError:
            return complex(y)

Edit: The second except is fixed to be for AttributeError.

Last edited 4 years ago by ppurka (previous) (diff)

comment:4 Changed 4 years ago by eviatarbach

Yes, but I don't think it should work with parent as int or Integer. Your solution coerces anything it doesn't know what to do with into complex, which is probably not what we want it to do. Besides, there isn't really any reason it should receive int or Integer as a parent; a symbolic function's _evalf_ shouldn't ever get passed that.

With the patch, if float is the parent and it gets complex output then it coerces to complex, which makes sense since it is has the same precision and is also a Python builtin type. I think the scope of the patch should be to just fix this issue, which affects plotting as noted in the description.

comment:5 Changed 4 years ago by eviatarbach

Okay, this new patch returns the original error message instead of giving a cryptic one. So, for example, if Integer is given as a parent:

sage: utils.call(mpmath.log, -3.0r, parent=Integer)
TypeError: unable to coerce <type 'sage.rings.complex_number.ComplexNumber'> to an integer
sage: utils.call(mpmath.log, -10.0r, parent=int)
TypeError: can't convert complex to int; use int(abs(z))

What do you think of this solution? I think it makes the most sense.

Changed 4 years ago by eviatarbach

comment:6 Changed 4 years ago by eviatarbach

Patchbot apply trac14984_2.patch

comment:7 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:8 Changed 3 years ago by rws

  • Authors set to Eviatar Bach
  • Branch set to public/14984
  • Commit set to bdcc06d6025a4c250ac1110c279a3ad14c20c7c8
  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

It works and I think this is fine. Tests in plot/ pass.


New commits:

bdcc06dTrac 14984: Cannot plot functions that use mpmath if complex numbers occur in the image

comment:9 Changed 3 years ago by vbraun

  • Branch changed from public/14984 to bdcc06d6025a4c250ac1110c279a3ad14c20c7c8
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.