Opened 9 years ago

Closed 9 years ago

#14984 closed defect (fixed)

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

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

Status badges

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 Eviatar Bach 9 years ago.
trac14984_2.patch (2.0 KB) - added by Eviatar Bach 9 years ago.

Download all attachments as: .zip

Change History (11)

Changed 9 years ago by Eviatar Bach

Attachment: trac14984.patch added

comment:1 Changed 9 years ago by Eviatar Bach

Component: coercionnumerical
Status: newneeds_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 9 years ago by Eviatar Bach (previous) (diff)

comment:2 Changed 9 years ago by Karl-Dieter Crisman

Cc: Karl-Dieter Crisman added

comment:3 Changed 9 years ago by Punarbasu Purkayastha

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 9 years ago by Punarbasu Purkayastha (previous) (diff)

comment:4 Changed 9 years ago by Eviatar Bach

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 9 years ago by Eviatar Bach

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 9 years ago by Eviatar Bach

Attachment: trac14984_2.patch added

comment:6 Changed 9 years ago by Eviatar Bach

Patchbot apply trac14984_2.patch

comment:7 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:8 Changed 9 years ago by Ralf Stephan

Authors: Eviatar Bach
Branch: public/14984
Commit: bdcc06d6025a4c250ac1110c279a3ad14c20c7c8
Reviewers: Ralf Stephan
Status: needs_reviewpositive_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 9 years ago by Volker Braun

Branch: public/14984bdcc06d6025a4c250ac1110c279a3ad14c20c7c8
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.