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

charpoly error on matrices with pi #13711

Closed
eviatarbach opened this issue Nov 15, 2012 · 18 comments
Closed

charpoly error on matrices with pi #13711

eviatarbach opened this issue Nov 15, 2012 · 18 comments

Comments

@eviatarbach
Copy link

Sage returns an error when attempting to calculate the characteristic polynomial of a matrix with pi:

sage: matrix([[sqrt(2), -1], [1, e^2]]).charpoly()    
x^2 + (-sqrt(2) - e^2)*x + sqrt(2)*e^2 + 1
sage: matrix([[sqrt(2), -1], [pi, e^2]]).charpoly()
TypeError

This is fixed in sage-6.6. The branch just add a doctest to the method charpoly of symbolic matrices.

Component: linear algebra

Author: Vincent Delecroix

Branch/Commit: 005cd02

Reviewer: Jeroen Demeyer

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

@eviatarbach
Copy link
Author

comment:1

Okay, I think I found the source of the error; Sage tries to coerce the pyobjects of the symbolic constants into the Symbolic Ring, which doesn't work:

sage: SR(pi.pyobject())
TypeError
sage: SR(euler_gamma.pyobject())
TypeError

What would be the best way to fix this? It seems to be quite a major problem, since characteristic polynomials cannot at the moment be computed for any matrices with symbolic constants (it does work for e, since it is defined differently, and golden_ratio, since it is replaced with its numerical value before running into this error).

@nbruin
Copy link
Contributor

nbruin commented Apr 24, 2013

comment:2

The right way of turning a constant into an expression is apparently via the expression method. The element constructor for SR tests for the presence of a _symbolic_ method on its argument. If we add a method to sage.symbolic.constants.Constant:

    def _symbolic_(self,SR):
        """
        doctest:
            sage: SR(sage.symbolic.constants.Constant('a'))
            a
            sage: SR(sage.symbolic.constants.Constant('a')).subs(a=10)
            a
        """
        return SR(self.expression())

the system picks up the conversion. The double conversion that's happening here is a little sad, but the interface for _symbolic_ is that the ring in question is given as a parameter. I guess our design does not rule out the existence of multiple symbolic rings, whereas expression will return a member of whatever symbolic ring is considered "home" by pynac.

One solution would be to give sage.symbolic.constants_c.PynacConstant.expression an optional argument SR to specify in which ring the expression should lie (defaulting to what SR is there now). This routine now just reads:

    def expression(self):
       return new_Expression_from_GEx(SR, <GEx>(self.pointer[0]))

so making that def expression(self, SR=SR) shouldn't make too much difference. With that in place we can just alias _symbolic_ to expression.

@nbruin
Copy link
Contributor

nbruin commented Apr 24, 2013

Attachment: trac_13711.patch.gz

ensure that SymbolicConstant object convert into SR

@nbruin
Copy link
Contributor

nbruin commented Apr 24, 2013

comment:4

Note that once you get a characteristic polynomial, it's likely wrong. See #14403

@eviatarbach
Copy link
Author

comment:5

Great! The patch works for me, and all tests pass on constants_c.pyx and constants.py.

@eviatarbach
Copy link
Author

comment:6

Maybe it would be good to add doctests for matrix([pi]).charpoly() and SR(pi.pyobject())?

@kcrisman
Copy link
Member

Author: Nils Bruin

@kcrisman
Copy link
Member

Reviewer: Eviatar Bach

@kcrisman
Copy link
Member

comment:7

Maybe it would be good to add doctests for matrix([pi]).charpoly() and SR(pi.pyobject())?

Agreed.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@videlec
Copy link
Contributor

videlec commented Apr 8, 2015

comment:12

Just works fine on 6.6-rc2

sage: sage: matrix([[sqrt(2), -1], [pi, e^2]]).charpoly()
x^2 + (-sqrt(2) - e^2)*x + pi + sqrt(2)*e^2

Migt be because symbolic constants are now element of the symbolic ring:

sage: type(pi)
<type 'sage.symbolic.expression.Expression'>
sage: pi.pyobject()
pi
sage: type(_)
<class 'sage.symbolic.constants.Pi'>

Vincent

@videlec videlec modified the milestones: sage-6.4, sage-6.6 Apr 8, 2015
@videlec
Copy link
Contributor

videlec commented Apr 14, 2015

Changed author from Nils Bruin to Vincent Delecroix

@videlec
Copy link
Contributor

videlec commented Apr 14, 2015

Branch: public/13711

@videlec
Copy link
Contributor

videlec commented Apr 14, 2015

Changed reviewer from Eviatar Bach to none

@videlec
Copy link
Contributor

videlec commented Apr 14, 2015

Commit: 005cd02

@videlec
Copy link
Contributor

videlec commented Apr 14, 2015

New commits:

005cd02Trac 13711: add a doctest to the method charpoly

@videlec

This comment has been minimized.

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Apr 15, 2015

Changed branch from public/13711 to 005cd02

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

8 participants