prepare deprecation of simple and alternative - #507
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #507 +/- ##
==========================================
- Coverage 86.93% 83.04% -3.90%
==========================================
Files 41 42 +1
Lines 2565 2671 +106
==========================================
- Hits 2230 2218 -12
- Misses 335 453 +118 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
* logging of Sidi * typos
* give regula falsi methods some attention; add Diaz and Perez framework * simplify nameing * add to docs * typo, italics * another typo
…gged (JuliaMath#516) closes JuliaMath#515 * adds a check on f being close to zero for bracketing intervals that throw `:not_converged`. In an earlier PR, this was a bug as this flag was not checked at all.
* add NonStrictBracketing type * engineer around stuck points in RegulaFalsi; clean up tests; reorganize decide_convergence
* update docs * doc edits; better engineering on regula falsi methods
Co-authored-by: jverzani <421708+jverzani@users.noreply.github.com>
… (keep existing compat) (JuliaMath#509) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org>
…7, (keep existing compat) (JuliaMath#510) Co-authored-by: CompatHelper Julia <compathelper_noreply@julialang.org>
There was a problem hiding this comment.
Pull request overview
This PR refactors Roots.jl’s “simple” and “alternative interfaces” code paths in preparation for deprecation by moving legacy APIs under src/deprecated/, relocating some implementations (e.g., bisection, dfree) into find_zeros.jl, and updating tests/docs accordingly. It also introduces a new RegulaFalsi bracketing family and adjusts convergence behavior for a new “non-strict bracketing” method category.
Changes:
- Move legacy
simple.jl/alternative_interfaces.jlintosrc/deprecated/and reorganize tests to match the deprecation plan. - Add
RegulaFalsiimplementation and new abstract method categories (AbstractRegulaFalsiMethod,AbstractNonStrictBracketingMethod) with associated convergence/tolerance behavior. - Update documentation, allocations tests, and test harness ordering (including running
Aqua.test_all(Roots)within a@testset), plus a downstream CI workflow.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_simple.jl | Consolidates “simple”/legacy interface tests; adds allocation tests for simple APIs. |
| test/test_newton.jl | Removes duplicated Newton/Halley interface tests now moved elsewhere. |
| test/test_fzero.jl | Adds regression test for passing a method through fzero. |
| test/test_find_zeros.jl | Adds tests/benchmarks for dfree and bisection moved under find_zeros. |
| test/test_find_zero.jl | Removes fzero regression test (relocated to test_fzero.jl). |
| test/test_derivative_free_interactive.jl | Fixes spelling for “diagnostics” helper naming. |
| test/test_composable.jl | Adjusts composability tests for bracketing/derivative-free methods. |
| test/test_bracketing.jl | Expands and restructures bracketing-method test coverage (incl. regula falsi variants). |
| test/test_allocations.jl | Updates allocation checks to include additional bracketing methods. |
| test/runtests.jl | Reorders includes, adds Aqua testset, and moves deprecated tests to a “deprecated” section. |
| src/trace.jl | Updates docs/examples for Tracks() keyword usage. |
| src/Roots.jl | Moves legacy includes into src/deprecated/ and adjusts exports; includes new bracketing file. |
| src/find_zeros.jl | Pulls in bisection and dfree implementations for broader use. |
| src/DerivativeFree/sidi.jl | Adjusts Sidi method initialization and function-call accounting. |
| src/deprecated/simple.jl | Removes duplicated bisection, adds legacy modab implementation and deprecation stubs. |
| src/deprecated/alternative_interfaces.jl | Refactors fzero legacy surface API and prepares deprecation points. |
| src/convergence.jl | Extends convergence checks to support regula falsi / non-strict bracketing categories; tweaks convergence decisions. |
| src/Bracketing/ridders.jl | Reclassifies Ridders as non-strict bracketing. |
| src/Bracketing/regula_falsi.jl | Adds new RegulaFalsi method family implementation. |
| src/Bracketing/modAB.jl | Improves references section in docs. |
| src/Bracketing/itp.jl | Reclassifies ITP as non-strict bracketing. |
| src/Bracketing/false_position.jl | Reclassifies FalsePosition under regula falsi umbrella and simplifies reductions. |
| src/Bracketing/chandrapatlu.jl | Reclassifies Chandrapatla as non-strict bracketing. |
| src/Bracketing/brent.jl | Reclassifies Brent as non-strict bracketing. |
| src/Bracketing/bracketing.jl | Adds default tolerances for non-strict bracketing methods. |
| src/Bracketing/alefeld_potra_shi.jl | Reclassifies Alefeld/Potra/Shi methods as non-strict bracketing. |
| src/abstract_types.jl | Introduces new abstract method categories for regula falsi and non-strict bracketing. |
| Project.toml | Bumps version and expands IntervalRootFinding compatibility. |
| docs/src/roots.md | Edits narrative and doctests to align with updated naming/examples. |
| docs/src/reference.md | Adds Roots.RegulaFalsi to reference list and updates method tables/descriptions. |
| docs/src/index.md | Updates package overview text for new/renamed method families. |
| .github/workflows/downstream.yml | Adds downstream integration test workflow against an external package. |
Suppressed comments (3)
src/deprecated/alternative_interfaces.jl:78
- When
verbose=true, this currently disables tracking for the defaulttracks=NullTracks()case, and overwrites user-provided tracks when they pass a non-NullTracksvalue. This is the opposite of the intended behavior (see the otherfzerooverloads below).
src/deprecated/simple.jl:111 - This prepared depwarn line uses the wrong symbol (
:a42), which will make the warning harder to filter and inconsistent once uncommented. It should match the function name (:modab).
test/test_simple.jl:58 - This check is currently not asserted (missing
@test), so it won’t catch regressions.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| `AlefeldPotraShi`, `Roots.Brent`, `Roots.Chandrapatlu`, | ||
| `Roots.ITP`, `Roots.Ridders`, `Roots.ModAB`, and ``12``-flavors of | ||
| `FalsePosition`. The default bracketing method for | ||
| `FalsePosition` and a few in `RegularFalsi`. The default bracketing method for |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
src/find_zeros.jl:134
dfree(f, xs)assumesxsis indexable and callslength(xs), but the public API (and the docstring example just above) passes a scalar initial guess likedfree(f, 1.0). That will throw a MethodError forlength(::Float64)unlessxsis normalized to a 1-tuple first.
function dfree(f, xs)
if length(xs) == 1
a = float(xs[1])
fa = f(a)
src/deprecated/simple.jl:29
- The planned deprecation message for
a42points users toRoots.AlefeldPotraShi(), buta42corresponds toRoots.A42()(as referenced in this file’s docstring). The message will be misleading once depwarns are enabled.
This issue also appears in the following locations of the same file:
- line 109
- line 320
src/deprecated/simple.jl:112
- The planned deprecation warning for
modabuses the symbol:a42, which would attribute the warning to the wrong binding. This should match the deprecated function name (e.g.:modab).
src/deprecated/simple.jl:330 - The planned deprecation warning for
mullerreferencesRoots.Muller(), but there is noMullermethod type in the codebase (only themullerfunction in this deprecated file). If/when depwarns are enabled, this message will direct users to a non-existent API.
src/convergence.jl:220 - Docstring bullet formatting:
* :exact_zerorenders oddly in Markdown (the `:` is outside the code span). It should be `* `:exact_zeroto match the other bullets.
If algorithm has stopped or converged, return flag and `true`. Flags are:
* :`exact_zero` if `|f(xn1)| == 0`
* `:x_converged` if `xn1 ≈ xn`, typically with non-zero tolerances specified.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This PR does:
fzeroThe goal is to deprecate the two files, both could be elsewhere, especially alternative interfaces which has had comments about it being legacy for years now.