Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
eviatarbach opened this issue Jul 30, 2013 · 12 comments
Closed

Comments

@eviatarbach
Copy link

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'

CC: @kcrisman

Component: numerical

Author: Eviatar Bach

Branch/Commit: bdcc06d

Reviewer: Ralf Stephan

Issue created by migration from https://trac.sagemath.org/ticket/14984

@eviatarbach eviatarbach added this to the sage-6.1 milestone Jul 30, 2013
@eviatarbach
Copy link
Author

comment:1

Attachment: trac14984.patch.gz

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

@ppurka
Copy link
Member

ppurka commented Aug 3, 2013

comment:3

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.

@eviatarbach
Copy link
Author

comment:4

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.

@eviatarbach
Copy link
Author

comment:5

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.

@eviatarbach
Copy link
Author

Attachment: trac14984_2.patch.gz

@eviatarbach
Copy link
Author

comment:6

Patchbot apply trac14984_2.patch

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@rwst
Copy link

rwst commented Apr 5, 2014

Branch: public/14984

@rwst
Copy link

rwst commented Apr 5, 2014

Author: Eviatar Bach

@rwst
Copy link

rwst commented Apr 5, 2014

Commit: bdcc06d

@rwst
Copy link

rwst commented Apr 5, 2014

Reviewer: Ralf Stephan

@rwst
Copy link

rwst commented Apr 5, 2014

comment:8

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

@vbraun
Copy link
Member

vbraun commented Apr 7, 2014

Changed branch from public/14984 to bdcc06d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants