Opened 9 years ago

Closed 6 years ago

# charpoly error on matrices with pi

Reported by: Owned by: eviatarbach jason, was major sage-6.6 linear algebra Vincent Delecroix Jeroen Demeyer N/A 005cd02 005cd02119165e28b2c7a9ae784e447289cfe859

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.

### comment:1 Changed 8 years ago by eviatarbach

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).

### comment:2 Changed 8 years ago by nbruin

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`.

Last edited 8 years ago by nbruin (previous) (diff)

### Changed 8 years ago by nbruin

ensure that SymbolicConstant? object convert into SR

### comment:3 Changed 8 years ago by nbruin

• Status changed from new to needs_review

### comment:4 Changed 8 years ago by nbruin

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

### comment:5 Changed 8 years ago by eviatarbach

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

Last edited 8 years ago by eviatarbach (previous) (diff)

### comment:6 follow-up: ↓ 7 Changed 8 years ago by eviatarbach

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 kcrisman

• Authors set to Nils Bruin
• 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()` and `SR(pi.pyobject())`?

Agreed.

### comment:8 Changed 8 years ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

### comment:9 Changed 8 years ago by vbraun_spam

• Milestone changed from sage-6.1 to sage-6.2

### comment:10 Changed 7 years ago by vbraun_spam

• Milestone changed from sage-6.2 to sage-6.3

### comment:11 Changed 7 years ago by vbraun_spam

• Milestone changed from sage-6.3 to sage-6.4

### comment:12 Changed 6 years ago by vdelecroix

• 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 vdelecroix

• Authors changed from Nils Bruin to Vincent Delecroix
• 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 jdemeyer

• Reviewers set to Jeroen Demeyer
• Status changed from needs_review to positive_review

### comment:15 Changed 6 years ago by vbraun

• Branch changed from public/13711 to 005cd02119165e28b2c7a9ae784e447289cfe859
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.