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: sage-6.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:


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 jdemeyer

  • Cc gagern added

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 5 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 5 years ago by gagern

  • Authors set to Martin von Gagern
  • 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:

472c754Implement symbolic (radical) roots of polynomials.

comment:5 Changed 5 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 5 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.