Opened 5 years ago
Closed 5 years ago
#17517 closed enhancement (fixed)
Implement roots of polynomials over SR
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage6.5 
Component:  number theory  Keywords:  
Cc:  gagern  Merged in:  
Authors:  Martin von Gagern  Reviewers:  Ralf Stephan 
Report Upstream:  N/A  Work issues:  
Branch:  472c754 (Commits)  Commit:  472c754f6b833011877484e0ce944a5d2ad9d3e0 
Dependencies:  Stopgaps: 
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
.
Change History (6)
comment:1 Changed 5 years ago by
 Cc gagern added
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
 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 5 years ago by
 Commit set to 472c754f6b833011877484e0ce944a5d2ad9d3e0
 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 5 years ago by
 Reviewers set to Ralf Stephan
 Status changed from needs_review to positive_review
Easy ticket, passes make ptestlong
.
comment:6 Changed 5 years ago by
 Branch changed from u/gagern/ticket/17517 to 472c754f6b833011877484e0ce944a5d2ad9d3e0
 Resolution set to fixed
 Status changed from positive_review to closed
I'm not sure that converting roots to
QQbar
first makes much sense. As it is now, conversion fromQQbar
toSR
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.