Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve refine_root() #19362

Open
jdemeyer opened this issue Oct 6, 2015 · 27 comments
Open

Improve refine_root() #19362

jdemeyer opened this issue Oct 6, 2015 · 27 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Oct 6, 2015

In particular, allow both real and complex input. Also implement bisection if Newton-Raphson cannot be used.

Component: algebra

Author: Jeroen Demeyer

Branch/Commit: u/jdemeyer/ticket/19362 @ 9d5f99c

Issue created by migration from https://trac.sagemath.org/ticket/19362

@jdemeyer jdemeyer added this to the sage-6.9 milestone Oct 6, 2015
@jdemeyer
Copy link
Author

jdemeyer commented Oct 6, 2015

Branch: u/jdemeyer/ticket/19362

@jdemeyer
Copy link
Author

jdemeyer commented Oct 6, 2015

Commit: 3b34e49

@jdemeyer
Copy link
Author

jdemeyer commented Oct 6, 2015

New commits:

2049f5aMove refine_root() to refine_root.pyx
3b34e49Improve refine_root()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

950eb84Implement bisection

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2015

Changed commit from 3b34e49 to 950eb84

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 6, 2015

comment:5
sage: from sage.rings.polynomial.refine_root import refine_root
sage: RIF = RealIntervalField(106)
sage: R.<x> = RIF[]
sage: pol = 10*x^6 - 10*x^2 + 1
sage: refine_root(pol, pol.derivative(), RIF(3/4,9/8), RIF)
Traceback (most recent call last):
...
ArithmeticError: cannot refine polynomial root (not enough steps?)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

972f8e0Once converging, keep converging

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2015

Changed commit from 950eb84 to 972f8e0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

eb61557When smashing, stop converging

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2015

Changed commit from 972f8e0 to eb61557

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2015

Changed commit from eb61557 to 7c689b6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

7c689b6When converging, take the intersection of old and new interval

@jdemeyer
Copy link
Author

jdemeyer commented Oct 7, 2015

comment:10

I am going to continue working on this a bit more. If anybody feels like reviewing this code, let me know and I'll put up the current version for review.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 7, 2015

Changed dependencies from #19361 to #19361, #19364

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

388d495Add edges() and endpoints() methods to intervals
8c10c00Merge branch 't/19364/add_edges___method_to_rif_and_cif_elements' into t/19362/ticket/19362
8ab38c4Trim instead of bisect

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2015

Changed commit from 7c689b6 to 8ab38c4

@videlec
Copy link
Contributor

videlec commented Oct 8, 2015

comment:13

Did you notice that some code in QQbar actually duplicates this: method _real_refine_interval/_complex_refine_interval of the ANRoot class.

@jdemeyer
Copy link
Author

jdemeyer commented Oct 8, 2015

comment:14

Replying to @videlec:

Did you notice that some code in QQbar actually duplicates this: method _real_refine_interval/_complex_refine_interval of the ANRoot class.

Yes, there are way too many re-implementations of this in Sage. My eventual goal is to replace all "real/complex root refining" code in Sage by this new refine_root() function. But I'm not there yet...

@jdemeyer
Copy link
Author

jdemeyer commented Oct 8, 2015

comment:15

Making it support all use cases of the various existing implementations is also what makes it tricky.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

9d5f99cImprove convergence check

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2015

Changed commit from 8ab38c4 to 9d5f99c

@JohnCremona
Copy link
Member

comment:17

What is the work which needs doing?

@jdemeyer
Copy link
Author

comment:18

I don't really remember myself, I do remember that it wasn't ready. It was trickier than I initially estimated. I will need to look at it again.

@jdemeyer
Copy link
Author

Changed dependencies from #19361, #19364 to none

@jdemeyer jdemeyer modified the milestones: sage-6.10, sage-7.1 Feb 18, 2016
@mezzarobba
Copy link
Member

comment:19

You probably know that, but just in case: it is not strictly true that you cannot use the interval Newton method and must switch to bisection when the slope interval contains zero. Another option is to interpret 1/[-a,b] as a kind of ”projective interval” containing ∞ (which gives rise to two disjoint complex intervals after intersecting with the previous estimate, so you'll still need to deal with several pieces).

@videlec
Copy link
Contributor

videlec commented Sep 7, 2016

comment:20

IMHO, refine_root is a bad name. I would expect such function to refine an interval that is already known to contain a (possibly unique) root... What about isolating_interval_from_approximate_root or something similar?

On the other hand, there are some possible alternative in the real case that does not involve convergence of Newton algorithm (and might already be implemented elsewhere). Namely p has a unique root in an interval [a, b] if the following holds:

  1. the sign of sgn(p(a)) * sgn(p(b)) = -1
  2. the interval p'([a,b]) does not contain zero
    It is also possible to replace 2 with Descartes rules of sign (which is a more expensive).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants