Opened 8 years ago
Closed 6 years ago
#13711 closed defect (fixed)
charpoly error on matrices with pi
Reported by:  eviatarbach  Owned by:  jason, was 

Priority:  major  Milestone:  sage6.6 
Component:  linear algebra  Keywords:  
Cc:  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  005cd02 (Commits, GitHub, GitLab)  Commit:  005cd02119165e28b2c7a9ae784e447289cfe859 
Dependencies:  Stopgaps: 
Description (last modified by )
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 sage6.6. The branch just add a doctest to the method charpoly of symbolic matrices.
Attachments (1)
Change History (16)
comment:1 Changed 8 years ago by
comment:2 Changed 8 years ago by
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
.
comment:3 Changed 8 years ago by
 Status changed from new to needs_review
comment:4 Changed 8 years ago by
Note that once you get a characteristic polynomial, it's likely wrong. See #14403
comment:5 Changed 8 years ago by
Great! The patch works for me, and all tests pass on constants_c.pyx
and constants.py
.
comment:6 followup: ↓ 7 Changed 8 years ago by
Maybe it would be good to add doctests for matrix([pi]).charpoly()
and SR(pi.pyobject())
?
comment:7 in reply to: ↑ 6 Changed 8 years ago by
 Reviewers set to Eviatar Bach
 Status changed from needs_review to needs_work
Maybe it would be good to add doctests for
matrix([pi]).charpoly()
andSR(pi.pyobject())
?
Agreed.
comment:8 Changed 8 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:9 Changed 7 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:10 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:11 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:12 Changed 6 years ago by
 Milestone changed from sage6.4 to sage6.6
Just works fine on 6.6rc2
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
comment:13 Changed 6 years ago by
 Branch set to public/13711
 Commit set to 005cd02119165e28b2c7a9ae784e447289cfe859
 Description modified (diff)
 Reviewers Eviatar Bach deleted
 Status changed from needs_work to needs_review
New commits:
005cd02  Trac 13711: add a doctest to the method charpoly

comment:14 Changed 6 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:15 Changed 6 years ago by
 Branch changed from public/13711 to 005cd02119165e28b2c7a9ae784e447289cfe859
 Resolution set to fixed
 Status changed from positive_review to closed
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:
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, andgolden_ratio
, since it is replaced with its numerical value before running into this error).