-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add bisection root-finder in arbitrary dimension #7
Conversation
Moved from JuliaIntervals/ValidatedNumerics.jl#262. |
Any comments, @lbenet? |
Codecov Report
@@ Coverage Diff @@
## master #7 +/- ##
=========================================
- Coverage 91.08% 82.18% -8.9%
=========================================
Files 4 5 +1
Lines 157 174 +17
=========================================
Hits 143 143
- Misses 14 31 +17
Continue to review full report at Codecov.
|
If you don't have any problem, I'm planning to merge this @lbenet. |
I will review it now; if I don't come back to you before lunch time, simple go ahead.... Sorry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ready to be merged; the change in Root
is not so dramatic. I've included one optional comment only.
Again, sorry for the long delay...
examples/complex_roots.jl
Outdated
""" | ||
function g(X::IntervalBox) | ||
x, y = X | ||
z = x + im*y; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really transcendental, but I read elsewhere that this is better written as z = complex(x, y)
(avoids one multiplication and one addition).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it maybe even better as z = complex( X... )
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, thanks.
I would like to add It's not the best name, but I can't think of a better one right now, and it's pretty nice that it's so easy to write. |
I agree with you. |
LGTM. I'll wait to travis before merging. |
@@ -0,0 +1,34 @@ | |||
using ValidatedNumerics, ValidatedNumerics.RootFinding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not in test/REQUIRE
and this file isn't getting run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks.
This adds a bisection root finder that works with single or multiple variables.
Currently, this involves modifying the
Root
type.For bisection in particular, this is, in some sense, not necessary, since it cannot prove uniqueness, so any roots are
:unknown
.However, for multi-dimensional root finding in general, it is maybe necessary to do so.