Add Brent's Method for root finding (numerical analysis) - #13063
Conversation
There was a problem hiding this comment.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper reviewto trigger the checks for only added pull request files@algorithms-keeper review-allto trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…into brent_method
for more information, see https://pre-commit.ci
|
Hi @mindaugl , can the PR be merged or are there any reviews pending? |
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Reviewed as part of the maths/ triage (#15081). This is a genuinely new algorithm — the repo has bisection, secant_method, and newton_raphson under maths/numerical_analysis/ but no Brent's method, so it fills a real gap. CI is green (build, ruff, pre-commit, docs) and I verified it converges: the two doctests pass, plus a battery of 2,383 random bracketed cubics and stress cases (flat cubic root (x-0.5)**3, x**5-0.3, transcendental cos(x)-x, sin(x) near π) all return roots with residual < 1e-6, 0 failures.
Two things before merge:
-
Placement (needs change): every other root-finder lives in
maths/numerical_analysis/(bisection.py,secant_method.py,newton_raphson.py). Please move this tomaths/numerical_analysis/brent_method.pyso it sits with its siblings andDIRECTORY.mdgroups it correctly. -
Robustness note (optional, non-blocking): the classic algorithm tracks a boolean
mflag(was the previous step a bisection?) to decide when to accept an interpolation step. This PR usesiteration <= 1/iteration > 1as a proxy instead. Bracketing is still preserved on every step by theif fl * fs < 0update, so correctness/termination hold (confirmed by the fuzz run), but the iteration-index proxy can accept an interpolation step the realmflagwould reject, which only costs a few extra iterations in adversarial cases. Worth a comment noting the deviation, or switching to a realmflagif you want the textbook behavior — either is fine.
Verdict: ready to merge once moved into numerical_analysis/. Nice, well-documented contribution.
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Reviewed as part of the maths/ awaiting-reviews triage (#15081).
Verdict: genuinely new + correct — recommend merge after one relocation.
Not a duplicate. The repo's bracketing/root-finders all live in maths/numerical_analysis/ (bisection.py, bisection_2.py, newton_raphson.py, secant_method.py). There's no Brent's method today, and it's a worthwhile addition — it's the standard "combine bisection + secant + inverse-quadratic-interpolation" method that scipy.optimize.brentq implements.
Correctness — verified empirically. I ran the implementation against scipy.optimize.brentq:
- All doctest/manual cases converge exactly (x³−x−2 → 1.52138, cos(x)−x → 0.73909, x²−612, eˣ−5, etc.).
- 2271 random bracketed polynomial roots (degree 2–6): 0 failures vs
brentq(|f(root)| < 1e-4).
One change requested before merge:
- Move the file to
maths/numerical_analysis/brent_method.pyso it sits with the other root-finders instead of at themaths/root. (DIRECTORY.mdwill regenerate.)
Two optional polish notes (non-blocking):
2. The mflag safeguard in canonical Brent tracks whether the previous step was a bisection; here it's approximated by iteration <= 1 / iteration > 1. Empirically it still converges on every case I threw at it, but a true boolean mflag (set when the bisection fallback fires, cleared otherwise) matches the reference algorithm exactly and is easier for readers to verify against Wikipedia/Numerical Recipes.
3. d, c = c, right reassigns d before it's read in the next iteration's abs(c - d) branch — the logic works out, but a one-line comment on what d (the second-previous iterate) represents would help future readers.
Nice, well-documented contribution — the parameter docstring and the sign-check ValueError doctest are exactly the style we want.
Description:
This PR adds Brent’s Method for root finding, a robust numerical algorithm combining bisection, secant, and inverse quadratic interpolation. The implementation includes:
References:
Fixes #13047
Describe your change:
Checklist: