Skip to content

Commit 7324957

Browse files
Fix bounds transform for problems with initialization data and quasi-Newton algorithms (#955)
* Fix bounds transform for problems with initialization data and quasi-Newton algs Two bugs in the box-constraint bounds transform: 1. `transform_bounded_problem` unconditionally read `alg.autodiff`, which threw a `FieldError` for algorithms without that field (e.g. `QuasiNewtonAlgorithm`, reached as part of the default polyalgorithm). Guard the access. 2. The wrapped function carried over the original `initialization_data`, which is defined in the *bounded* coordinates. Running it on the transformed problem interpreted the unbounded iterate `t` as a bounded state and corrupted the solution (e.g. ModelingToolkit `NonlinearProblem`s, which always attach initialization data, converged to `from_unbounded(root)` instead of `root`). Initialization is now run once in the original bounded space before transforming, and stripped from the transformed function so it isn't re-run in `t`-space. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> * Test the bounds-transform fix in NonlinearSolveBase's own suite The umbrella `test/bounds_tests.jl` integration test exercised the fix through a full solve, but on Julia < 1.11 the umbrella resolves the *registered* NonlinearSolveBase (the `[sources]` path redirect to the in-repo sublibrary is ignored there), so the test ran against unpatched code and failed. Add a `transform_bounded_problem` unit test to NonlinearSolveBase's own test suite, which always runs against the in-repo code (incl. on lts), covering the quasi-Newton `autodiff`-access guard with a dummy algorithm that has no `autodiff` field. Gate the umbrella integration test on the loaded NonlinearSolveBase version so it only runs where the fix is present. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> --------- Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
1 parent 0f7f1cd commit 7324957

5 files changed

Lines changed: 91 additions & 4 deletions

File tree

lib/NonlinearSolveBase/Project.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
name = "NonlinearSolveBase"
22
uuid = "be0214bd-f91f-a760-ac4e-3421ce2b2da0"
3-
version = "2.30.2"
3+
version = "2.30.3"
44
authors = ["Avik Pal <avikpal@mit.edu> and contributors"]
55

66
[deps]

lib/NonlinearSolveBase/src/bounds_transform.jl

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,13 @@ end
117117
# unbounded parameter `t` using the logistic function to map all values of `t`
118118
# into the interval (lb, ub).
119119
function transform_bounded_problem(prob, alg)
120+
# `initialization_data` (e.g. from ModelingToolkit) is defined in the original,
121+
# bounded coordinates. It must run *before* the transform so the resulting `u0`/`p`
122+
# are consistent; running it on the transformed problem would interpret the unbounded
123+
# iterate `t` as a bounded state and corrupt the solution. We run it here and strip it
124+
# from the transformed function below so it isn't re-run in `t`-space.
125+
prob = run_bounded_initialization(prob, alg)
126+
120127
lb, ub = _normalize_bounds(prob.lb, prob.ub, prob.u0)
121128

122129
# Clamp u0 into the interior of the bounds so that _to_unbounded doesn't hit log(0)
@@ -125,8 +132,10 @@ function transform_bounded_problem(prob, alg)
125132
u0_transformed = _to_unbounded.(u0_clamped, lb, ub)
126133

127134
# PreallocationTools is only supported by ForwardDiff so we only use
128-
# FixedSizeDiffCache if we're using ForwardDiff.
129-
u_cache = if isnothing(alg) || isnothing(alg.autodiff) || alg.autodiff isa AutoForwardDiff
135+
# FixedSizeDiffCache if we're using ForwardDiff. Not every algorithm has an
136+
# `autodiff` field (e.g. `QuasiNewtonAlgorithm`), so guard the access.
137+
alg_ad = alg !== nothing && hasproperty(alg, :autodiff) ? alg.autodiff : nothing
138+
u_cache = if alg_ad === nothing || alg_ad isa AutoForwardDiff
130139
FixedSizeDiffCache(prob.u0)
131140
else
132141
similar(prob.u0)
@@ -148,7 +157,24 @@ function transform_bounded_problem(prob, alg)
148157
wrapped
149158
end
150159

151-
transformed_prob = remake(prob; f = new_f, u0 = u0_transformed, lb = nothing, ub = nothing)
160+
transformed_prob = remake(
161+
prob; f = new_f, u0 = u0_transformed, lb = nothing, ub = nothing,
162+
build_initializeprob = Val{false}
163+
)
152164

153165
return transformed_prob
154166
end
167+
168+
# Run problem initialization (if any) in the original bounded coordinates so that the
169+
# resulting `u0`/`p` are consistent before the bounds transform is applied. Returns the
170+
# problem unchanged when there is no `OverrideInitData` to run.
171+
function run_bounded_initialization(prob, alg)
172+
SciMLBase.has_initialization_data(prob.f) || return prob
173+
prob.f.initialization_data isa SciMLBase.OverrideInitData || return prob
174+
iip = SciMLBase.isinplace(prob)
175+
u0, p, _ = SciMLBase.get_initial_values(
176+
prob, prob, prob.f, SciMLBase.OverrideInit(), Val(iip);
177+
nlsolve_alg = alg, abstol = get_abstol(prob), reltol = get_reltol(prob)
178+
)
179+
return remake(prob; u0, p, build_initializeprob = Val{false})
180+
end
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
module BoundsTransformTests
2+
3+
using Test
4+
using NonlinearSolveBase
5+
using SciMLBase
6+
# Load ForwardDiff so PreallocationTools' `FixedSizeDiffCache` dual-cache extension is
7+
# active (the transform builds one for the default/ForwardDiff autodiff path).
8+
import ForwardDiff
9+
using NonlinearSolveBase: transform_bounded_problem, BoundedWrapper, _to_unbounded
10+
11+
# An algorithm type without an `autodiff` field, mirroring `QuasiNewtonAlgorithm`
12+
# (which the default polyalgorithm reaches). `transform_bounded_problem` must not
13+
# assume every algorithm exposes `autodiff`.
14+
struct NoAutodiffAlg end
15+
16+
@testset "bounds transform handles algorithms without an `autodiff` field" begin
17+
f(u, p) = u .^ 2 .- p
18+
prob = NonlinearProblem(f, [1.5, 1.5], [2.0, 2.0]; lb = [0.0, 0.0], ub = [3.0, 3.0])
19+
20+
for alg in (nothing, NoAutodiffAlg())
21+
tprob = transform_bounded_problem(prob, alg)
22+
@test tprob.f.f isa BoundedWrapper
23+
# bounds are removed from the transformed (unconstrained) problem
24+
@test tprob.lb === nothing
25+
@test tprob.ub === nothing
26+
# u0 is mapped into unbounded space
27+
@test tprob.u0 _to_unbounded.(prob.u0, prob.lb, prob.ub)
28+
end
29+
end
30+
31+
end

lib/NonlinearSolveBase/test/runtests.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,4 +142,8 @@ using InteractiveUtils, Test
142142
@testset "EnzymeExt algorithms are inactive_type" begin
143143
include("enzyme_inactive_algorithm.jl")
144144
end
145+
146+
@testset "Bounds transform (#955)" begin
147+
include("bounds_transform.jl")
148+
end
145149
end

test/bounds_tests.jl

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,29 @@ end
157157
end
158158
end
159159
end
160+
161+
@testitem "Bounds: polyalgorithm and quasi-Newton algorithms" tags = [:core, :bounds] begin
162+
using SciMLBase
163+
using NonlinearSolveBase
164+
165+
# The quasi-Newton `autodiff`-access fix lives in NonlinearSolveBase, so only
166+
# exercise the full solve when a NonlinearSolveBase new enough to contain it is
167+
# actually loaded. On Julia < 1.11 the umbrella resolves the *registered*
168+
# NonlinearSolveBase (which predates the fix) because `[sources]` path redirects
169+
# are ignored there; the fix itself is unit-tested in NonlinearSolveBase's own
170+
# suite, which always runs against the in-repo code. See SciML/NonlinearSolve.jl#955.
171+
if pkgversion(NonlinearSolveBase) >= v"2.30.3"
172+
# `x^2 - 4x + 3` has roots at 1 and 3; bounds select which root is reachable.
173+
f(u, p) = u .^ 2 .- 4 .* u .+ 3
174+
175+
# The default polyalgorithm tries quasi-Newton methods (which have no `autodiff`
176+
# field) as part of its sequence; the bounds transform must not error on them.
177+
for alg in (nothing, FastShortcutNonlinearPolyalg(), Broyden(), Klement(), NewtonRaphson())
178+
prob = NonlinearProblem(f, [1.5], nothing; lb = [0.0], ub = [2.0])
179+
sol = alg === nothing ? solve(prob) : solve(prob, alg)
180+
@test SciMLBase.successful_retcode(sol)
181+
@test sol.u[1] 1.0 atol = 1.0e-6
182+
@test 0.0 <= sol.u[1] <= 2.0
183+
end
184+
end
185+
end

0 commit comments

Comments
 (0)