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: | sage-6.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 sage-6.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 follow-up: ↓ 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 sage-5.11 to sage-5.12
comment:9 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:10 Changed 7 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:11 Changed 7 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:12 Changed 6 years ago by
- Milestone changed from sage-6.4 to sage-6.6
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
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).