Opened 5 years ago

Closed 4 years ago

# Implement roots of polynomials over SR

Reported by: Owned by: jdemeyer major sage-6.5 number theory gagern Martin von Gagern Ralf Stephan N/A 472c754 (Commits) 472c754f6b833011877484e0ce944a5d2ad9d3e0

### Description

The following should work:

```sage: R.<x> = QQ[]
sage: (x^2 + 2).roots(SR)
[-sqrt(-2), sqrt(-2)]
```

This could easily be done by first finding the roots in `QQbar` and then converting to `SR`.

### comment:2 Changed 5 years ago by gagern

I'm not sure that converting roots to `QQbar` first makes much sense. As it is now, conversion from `QQbar` to `SR` works by first finding all radical symbolic roots and then identifying the matching one. For this use case here, we wouldn't have to worry about that matching step. And we could avoid the computation for root isolation as well. Therefore, unless something changes dramatically in how these things work, I'd rather implement this conversion directly, and then perhaps have the radical expressions for algebraic numbers call that. If you agree, feel free to remove the dependency on #14239.

### comment:3 Changed 4 years ago by gagern

• Branch set to u/gagern/ticket/17517
• Created changed from 12/16/14 22:07:01 to 12/16/14 22:07:01
• Modified changed from 12/17/14 00:40:00 to 12/17/14 00:40:00

### comment:4 Changed 4 years ago by gagern

• Authors set to Martin von Gagern
• Dependencies #14239 deleted
• Status changed from new to needs_review

I decided to implement this without waiting on #17516. We can always add an extra code path for that later on. I consider this method the canonical place to compute radical expressions for polynomials, so I've adapted the approach via `Expression.solve` I've been using in other places as well. If this gets accepted quickly, I might even change #14239 to make use of this.

There is one situation I just thought of: if the coefficients are from `AA` or `QQbar`, then my current implementation will fail. In that situation your original suggestion, of computing roots in `QQbar` and then turning them into symbolic expressions, starts to make a lot of sense. But I'd rather handle that in a separate ticket, since such polynomials are much rarer and that code really would depend on #14239 which, as I just suggested, could benefit from depending on this implementation here.

New commits:

 ​472c754 `Implement symbolic (radical) roots of polynomials.`

### comment:5 Changed 4 years ago by rws

• Reviewers set to Ralf Stephan
• Status changed from needs_review to positive_review

Easy ticket, passes `make ptestlong`.

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

• Branch changed from u/gagern/ticket/17517 to 472c754f6b833011877484e0ce944a5d2ad9d3e0
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.