Skip to content

Fix strict Kantorovich step acceptance#3908

Merged
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:agent/implicit-discrete-homotopy
Jul 22, 2026
Merged

Fix strict Kantorovich step acceptance#3908
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:agent/implicit-discrete-homotopy

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Jul 15, 2026

Copy link
Copy Markdown
Member

Please ignore this draft until it has been reviewed by @ChrisRackauckas.

Summary

  • Fix strict KantorovichTypeController acceptance to accept a converged step only when every recorded contraction satisfies Θk <= Θreject.
  • Keep non-strict mode accepting every converged nonlinear solve.
  • Add boundary, rejection, empty-history, and non-strict regression coverage.
  • Bump ImplicitDiscreteSolve from 2.1.3 to 2.1.4.

Root cause

The controller documentation says strict mode rejects a step whenever any Θk > Θreject. The implementation used all(Θreject .< Θks), which reversed the inequality: normal well-contracted steps were rejected, while a history in which every contraction exceeded the rejection threshold was accepted.

Why this remains separate from homotopy solving

NonlinearSolve #1071 now provides the controller as KantorovichHomotopy for a genuine HomotopyProblem, and OrdinaryDiffEq's existing HomotopyNonlinearSolveAlg adapter can use it for ODE stage equations. That does not make it a drop-in replacement for this controller:

  • HomotopyNonlinearSolveAlg is an OrdinaryDiffEqCore.AbstractNLSolverAlgorithm consumed by the ODE stage-solver machinery, not a NonlinearSolve algorithm accepted by init(::NonlinearProblem, ...). An exact attempted substitution in IDSolve.nlsolve exits with SciMLBase.NonSolverError.
  • A general ImplicitDiscreteProblem solves f(u_next, uprev, p, t) = 0 and replaces uprev after every accepted state. It is therefore a sequence of changing nonlinear problems, not one fixed homotopy in t.
  • An artificial per-step affine homotopy can preserve the endpoint equation for square problems, but it changes the solve strategy, is not a generic construction for the existing rectangular least-squares cases, and does not match current performance.

Performance check

The closest generic square-problem prototype used

$$H(x, λ) = (1 - λ)(x - u_{prev}) + λ f(x, u_{prev}, p, t)$$

at every discrete step. The recurrence endpoint matched the current solver to about 1e-12. Medians below are from 20 warmed Julia 1.12.6 samples:

Recurrence implementation Time Residual calls Memory Allocations
current IDSolve 0.592 ms 360 65,904 B 1,357
affine KantorovichHomotopy per step 10.292 ms 1,358 3,288,096 B 47,965
affine HomotopySweep per step 10.444 ms 1,958 3,423,296 B 51,215

The Kantorovich replacement was 17.4x slower and allocated 35x as many objects.

For a scalar path whose residual ignores uprev, a whole-span homotopy is valid and avoids the IDS integrator overhead. In that restricted case, HomotopySweep took 0.205 ms and KantorovichHomotopy took 0.347 ms (current IDS: 0.507 ms), with all endpoints agreeing at floating-point precision. This is consistent with NonlinearSolve #1071's broader comparison: Kantorovich continuation can win on some paths, but it is not a general replacement for Sweep or for the recurrence controller here.

Validation

Run on the rebased branch head 6a73faf817:

  • ODEDIFFEQ_TEST_GROUP=Core on Julia 1.10.11: exit 0, all 34 assertions passed; Kantorovich strict acceptance passed 4/4.

  • Repository-wide Runic check: exit 0.

  • git diff --check: exit 0.

  • ODEDIFFEQ_TEST_GROUP=QA on rebased head: JET passed 1/1; Aqua passed 16/17 and failed only the rendered-public-API check on 185 SciMLBase re-exports plus IDSolve (186 names total). This clean-master failure was bisected to Update SciMLTesting QA docs coverage #3867 and remains isolated in #3969; that focused fix passes Julia 1.10 JET 1/1 and Aqua 17/17 plus Runic and diff checks. The umbrella docs fix #3970 is merged in this branch base. No test was skipped, silenced, loosened, or changed.

  • Rebased-head GitHub CI completed with 119 successful, 3 skipped, and 7 failed checks. All ImplicitDiscreteSolve Core lanes, its downgrade lane, documentation, Runic, and both benchmark lanes passed. The non-passing checks are independent clean-master baselines or infrastructure:

    • IDS rendered-API QA: #3969. The former root half is fixed by merged #3970.
    • MatrixOperator downgrade failures in BDF, SDIRK, and OrdinaryDiffEqNonlinearSolve: #3917, with shared-workflow prerequisite SciML/.github #118. The exact locked Raise NonlinearSolveBase floor for operator Jacobians #3917 Core suite was independently rerun on Julia 1.11 and exited 0, including the formerly failing 44/44 and 113/113 groups.
    • TaylorSeries resolver conflict: #3965, independently verified at 81/81 locked Core tests.
    • DiffEqDevTools: the self-hosted runner lost communication again; the job-specific rerun endpoint returned HTTP 403.
    • GPU: exited 1 after 22m23s, but the self-hosted failed-step log is unavailable. Equivalence to the independently reproduced clean-master 137-case DAE failure is therefore an inference, tracked by #3922, #3923, and CUDA.jl #3199; no NVIDIA GPU is available locally.

Process notes

The work mapped the exact residual-contraction sampling and Deuflhard step-size formula, extracted the reusable controller into NonlinearSolve, benchmarked it against HomotopySweep and ArcLengthContinuation, tested the proposed direct and affine adapters after NonlinearSolve #1071 merged, and kept this independent predicate bug as a small OrdinaryDiffEq patch.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Commit f32e1001cd contains the controller-only change on top of prerequisite #3907.

Executed plan and local evidence:

  1. Compared the documented predicate (reject if any Θk > Θreject) with the broadcasted implementation and isolated the reversed inequality.
  2. Added direct boundary coverage for empty contraction history, Θk == Θreject, the first value above the threshold, and non-strict acceptance.
  3. Ran the exact stacked branch through ODEDIFFEQ_TEST_GROUP=Core: every testset passed; the new testset was 4/4.
  4. Ran ODEDIFFEQ_TEST_GROUP=QA: JET 1/1, Aqua 18/18, package tests passed.
  5. Ran repository-wide Runic --check . and git diff --check: both exit 0.

No test was skipped, disabled, loosened, or converted to warn-only behavior.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

CI note: the red downgrade-sublibraries / test (lib/OrdinaryDiffEqCore) is a current-master baseline dependency-floor failure. This PR job and master job 87111089373 both pin SciMLOperators 1.23.0 while the in-tree OrdinaryDiffEqNonlinearSolve 2.3.0 requires 1.24.3. It fails during Pkg.test(...allow_reresolve=false) resolution before tests execute; neither the isdiscretecache API restoration nor the strict controller test participates. A separate clean-master floor investigation is queued; no test will be skipped or silenced.

@ChrisRackauckas-Claude
ChrisRackauckas-Claude force-pushed the agent/implicit-discrete-homotopy branch from f32e100 to b1f6c20 Compare July 15, 2026 10:20
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Rebased this PR directly onto current master and removed the unrelated #3907 commit. The branch now contains only b1f6c20788 (strict acceptance fix + regression test + patch bump). Re-ran locally on the exact branch: ODEDIFFEQ_TEST_GROUP=Core julia +1.10 --project=lib/ImplicitDiscreteSolve -e 'using Pkg; Pkg.test()' exited 0 with all 34 assertions passing, including Kantorovich strict acceptance 4/4; Runic on both changed Julia files and git diff --check also exited 0.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

CI note: the red downgrade jobs for lib/StochasticDiffEq (job 87329808979) and lib/OrdinaryDiffEqFIRK (job 87329808768) are the independently reproduced current-master source-floor conflict: the downgrade action pins SciMLOperators = 1.23, while the in-tree OrdinaryDiffEqNonlinearSolve 2.3.0 requires 1.24.3. Clean-master Julia 1.11 reproductions and the first introducing commit are being handled in a separate focused baseline investigation per repository policy. This strict-predicate PR changes only ImplicitDiscreteSolve; it will not mix in those monorepo dependency-floor repairs.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Feature-specific CI update: ImplicitDiscreteSolve Core now passes on Julia LTS, current stable, and prerelease; its downgrade job passes, as does SimpleImplicitDiscreteSolve downgrade. The sole red ImplicitDiscreteSolve check is QA, which is the independently reproduced current-master removal of the previously released OrdinaryDiffEqCore.isdiscretecache public declaration; the focused restoration is #3907. The controller branch remains limited to the strict-predicate fix.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Baseline dependency-floor update: the minimal registered-root prerequisite is now draft NonlinearSolve #1079. It patches NonlinearSolve to 4.21.1, permits LinearSolve 4.3/5, and raises the NonlinearSolveBase floor to the operator-capable 2.35. Exact local evidence includes FirstOrder with LinearSolve 4.3 (including operator tests 14/14), LinearSolve 5 operator tests 14/14, QA 15/15, Runic 373/373, and the formerly failing OrdinaryDiffEq nonlinear sets 44/44, 113/113, and 10/10. The focused OrdinaryDiffEq source-floor follow-up remains separate from this controller PR.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Investigated the failed downgrade-sublibraries / test (lib/DiffEqDevTools) lane.

CI evidence:

  • PR #3908 job 87329808771 ran on self-hosted-4vcpu-8gb-vv9dn-runner-8mjh8. Setup, checkout, Julia setup, resolve, and build succeeded. The test step started at 11:05:53Z, but the API still records that step as in_progress with no completion timestamp.
  • GitHub attached: “The self-hosted runner lost communication with the server. Verify the machine is running and has a healthy network connection. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error.”
  • The logs endpoint returns HTTP 404 BlobNotFound, so there is no Julia exception or failed assertion to inspect.
  • PR #3912 job 87336560102 repeated the same incomplete test-step metadata and lost-runner annotation on a different runner, self-hosted-4vcpu-8gb-vv9dn-runner-rq2hh. The two runners survived for different durations, about 44 and 22 minutes respectively.

Clean local reproduction from upstream master commit 5265a6c54232db89d6e57e7a2c69f5f20d55a172:

  • Julia 1.11.9, isolated depot and temp directory, the exact v2 downgrade action/skip set, and ODEDIFFEQ_TEST_GROUP=Core.
  • The downgrade resolution completed successfully in 2m42.20s, with maximum RSS 2,434,052 KiB.
  • The exact test command was Pkg.test(; allow_reresolve = false). A warm-depot definitive rerun exited 0 with Testing DiffEqDevTools tests passed.
  • Test wall time was 53m58.65s. Important summaries included Benchmark 15/15, Analyticless Convergence 7/7, Analyticless Stochastic WP 2/2, Stability Region 7/7, Plot Recipes 36/36, and NonlinearSolve WP diagrams 6/6.
  • Maximum RSS was 7,807,812 KiB (about 7.45 GiB), with zero swaps.

The local peak leaves only about 0.55 GiB on a nominal 8 GiB runner before the OS and runner service. The evidence is therefore consistent with runner memory pressure terminating the VM/runner connection, not a reproducible Julia source/test regression. I found no source failure to bisect and made no code change. This lane should be rerun on a larger-memory runner or split into lower-peak jobs.

@ChrisRackauckas-Claude
ChrisRackauckas-Claude force-pushed the agent/implicit-discrete-homotopy branch from b1f6c20 to 8b2950d Compare July 20, 2026 17:48
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Rebased the focused predicate fix onto current clean upstream/master (ed6bf86fcc, including #3907) and updated the branch with an explicit force-with-lease after auditing the remote branch. New head: 8b2950d957.

I also tested the proposed replacement with HomotopyNonlinearSolveAlg(KantorovichHomotopy(...)) rather than inferring compatibility from the names:

  • Direct substitution is not an accepted API path. IDSolve initializes a NonlinearProblem with alg.nlsolve, while HomotopyNonlinearSolveAlg <: OrdinaryDiffEqCore.AbstractNLSolverAlgorithm is an ODE-stage solver consumed by OrdinaryDiffEqNonlinearSolve.NLSolver. The exact attempted IDSolve(nlsolve = HomotopyNonlinearSolveAlg(HomotopySweep())) run exits with SciMLBase.NonSolverError before solving.
  • A whole-span HomotopyProblem is not a semantics-preserving replacement for a general ImplicitDiscreteProblem: after each accepted step the residual changes from f(u_next, uprev, p, t) to one with a new uprev. Only problems whose residual ignores uprev define a fixed path in t.
  • I benchmarked the closest generic square-problem adapter, the per-step affine embedding H(x, λ) = (1-λ)(x-uprev) + λ f(x, uprev, p, t), against the existing recurrence solver. All numbers are medians of 20 Julia 1.12.6 samples after warmup:
Recurrence implementation Time Residual calls Memory Allocations
current IDSolve 0.592 ms 360 65,904 B 1,357
affine KantorovichHomotopy per step 10.292 ms 1,358 3,288,096 B 47,965
affine HomotopySweep per step 10.444 ms 1,958 3,423,296 B 51,215

The Kantorovich adapter reached the same endpoint to about 1e-12, but was 17.4x slower and allocated 35x as many objects. It also does not extend generically to the existing rectangular least-squares cases without choosing an arbitrary rectangular anchor system.

For a scalar fixed path where a whole-span homotopy is valid, direct continuation beat the full IDS integrator overhead, but HomotopySweep was faster than KantorovichHomotopy: 0.205 ms versus 0.347 ms (current IDS: 0.507 ms), all reaching cbrt(2) to floating-point precision. This matches #1071's conclusion that Kantorovich continuation is useful but not a general replacement for Sweep.

Accordingly, this PR remains the small correctness fix for the general recurrence controller; no homotopy adapter code was added.

Post-rebase local validation:

  • ODEDIFFEQ_TEST_GROUP=Core on Julia 1.10.11: exit 0, all 34 assertions passed, including strict acceptance 4/4.
  • repository-wide Runic check: exit 0.
  • git diff --check: exit 0.
  • ODEDIFFEQ_TEST_GROUP=QA: JET passed 1/1, then public-doc rendering failed on 186 SciMLBase re-exports. The exact failure reproduces on untouched upstream/master ed6bf86fcc; the mandated separate clean-master bisect/fix investigation is running in parallel. No test was skipped, silenced, or changed.

Copy link
Copy Markdown
Member Author

One-hour rebased-head CI audit:

No completed check implicates the controller predicate or its regression tests. The PR description now contains the benchmark and this full baseline map.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas-Claude
ChrisRackauckas-Claude force-pushed the agent/implicit-discrete-homotopy branch from 8b2950d to 6a73faf Compare July 20, 2026 21:32
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Rebased after #3970 merged. Current base/head: 298da19f44 / 6a73faf817.

Fresh local validation on this exact head:

  • ODEDIFFEQ_TEST_GROUP=Core, Julia 1.10.11: exit 0, 34/34 assertions; strict acceptance 4/4.
  • repository-wide Runic 1.7: exit 0.
  • git diff --check: exit 0.
  • QA: JET 1/1; Aqua 16/17, with only the known 186-name IDS rendered-API baseline tracked by Fix ImplicitDiscreteSolve API docs QA #3969.

The branch was force-updated only after fetching and confirming the remote feature branch still contained solely the prior authored commit; the update used an explicit force-with-lease.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Fresh post-#3970 CI is complete: 119 success, 3 skipped, 7 failed.

All #3908-relevant checks passed: ImplicitDiscreteSolve Core on LTS/current/pre, IDS downgrade, documentation, Runic, and both benchmark lanes.

The seven failures are independently tracked/classified:

No tests were skipped, silenced, loosened, or changed.

@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review July 22, 2026 07:59
@ChrisRackauckas
ChrisRackauckas merged commit 8fc48e6 into SciML:master Jul 22, 2026
122 of 129 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants