From 8c775beefa6cd42f50955305152f82777e7e2749 Mon Sep 17 00:00:00 2001 From: JamesWrigley Date: Tue, 7 Jul 2026 14:38:39 +0200 Subject: [PATCH] Copy nonlinear fit input arrays into the cache struct So that we don't accidentally overwrite the input when using the caches directly and calling `reinit!()` on them. --- Project.toml | 2 +- docs/src/_changelog.md | 9 +++++++ src/common_interface.jl | 7 +++++ src/nonlinfit.jl | 56 ++++++++++++++++++---------------------- test/nonlinfit_reinit.jl | 12 +++++++++ 5 files changed, 54 insertions(+), 32 deletions(-) diff --git a/Project.toml b/Project.toml index 367e6f4..3c45c95 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "CurveFit" uuid = "5a033b19-8c74-5913-a970-47c3779ef25c" -version = "1.10.1" +version = "1.10.2" authors = ["Paulo José Saiz Jabardo , Avik Pal and contributors"] [deps] diff --git a/docs/src/_changelog.md b/docs/src/_changelog.md index 2c5fd53..dd2fd90 100644 --- a/docs/src/_changelog.md +++ b/docs/src/_changelog.md @@ -7,6 +7,15 @@ CurrentModule = CurveFit This documents notable changes in CurveFit.jl. The format is based on [Keep a Changelog](https://keepachangelog.com). +## [v1.10.2] - 2026-07-07 + +### Fixed +- Changed the solver for [`NonlinearCurveFitProblem`](@ref) to make copies of + the input arrays into its internal cache so that the original input arrays + won't be overwritten if `reinit!(cache; x, y, sigma)` is called + ([#117]). Doing one-off solves will incur extra allocations due to the copy, + reusing the cache remains as efficient as before. + ## [v1.10.1] - 2026-07-01 ### Fixed diff --git a/src/common_interface.jl b/src/common_interface.jl index 25b0c4c..3afaa5c 100644 --- a/src/common_interface.jl +++ b/src/common_interface.jl @@ -458,6 +458,13 @@ end Represents the solution to a curve fitting problem. This is a callable struct and can be used to evaluate the solution at a point. Exact evaluation mechanism depends on the algorithm used to solve the problem. + +!!! note "Aliasing semantics" + For nonlinear fits, the `x`/`y`/`sigma` arrays in `sol.prob` alias the + solver cache's internal arrays rather than copies of them to avoid + allocations. A subsequent `reinit!` of that cache mutates those arrays in + place, so a solution returned beforehand will reflect the new data. Copy + the `sol.prob` fields if you need a stable snapshot. """ @concrete struct CurveFitSolution <: AbstractCurveFitSolution alg <: AbstractCurveFitAlgorithm diff --git a/src/nonlinfit.jl b/src/nonlinfit.jl index 6408092..5d669c5 100644 --- a/src/nonlinfit.jl +++ b/src/nonlinfit.jl @@ -6,11 +6,6 @@ end SciMLBase.isinplace(::NonlinearFunctionWrapper{iip}) where {iip} = iip -_unwrap_nonlinear_function(f::NonlinearFunctionWrapper) = f -_unwrap_nonlinear_function(f::NonlinearSolveBase.AutoSpecializeCallable) = _unwrap_nonlinear_function(f.orig) -_unwrap_nonlinear_function(f::NonlinearSolveBase.BoundedWrapper) = _unwrap_nonlinear_function(f.f.f) -_unwrap_nonlinear_function(f) = f - __wrap_nonlinear_function(f::NonlinearFunction, ::Nothing, ::Nothing) = f function __wrap_nonlinear_function(f::NonlinearFunction, target, sigma) internal_f = NonlinearFunctionWrapper{SciMLBase.isinplace(f)}(target, sigma, f.f) @@ -54,6 +49,9 @@ end @concrete struct GenericNonlinearCurveFitCache <: AbstractCurveFitCache prob <: CurveFitProblem cache + x <: AbstractArray + y <: Union{AbstractArray, Nothing} + sigma <: Union{AbstractArray, Nothing} u0 alg kwargs @@ -68,23 +66,23 @@ function SciMLBase.reinit!(cache::GenericNonlinearCurveFitCache; u0 = nothing, x # x becomes `p` (parameter) in the NonlinearLeastSquaresProblem if !isnothing(x) - @assert size(x) == size(_get_cache(cache).p) "reiniting `x` must keep the same size" - kwargs = (; kwargs..., p = x) + @assert size(x) == size(cache.x) "reiniting `x` must keep the same size" + copyto!(cache.x, x) + kwargs = (; kwargs..., p = cache.x) end # Update `y` inplace - wrapper = _unwrap_nonlinear_function(cache.cache.prob.f.f) if !isnothing(y) - @assert wrapper isa NonlinearFunctionWrapper && !isnothing(wrapper.target) "cannot reinit `y` for a problem created without a `y`" - @assert size(y) == size(wrapper.target) "reiniting `y` must keep the same size" - copyto!(wrapper.target, y) + @assert !isnothing(cache.y) "cannot reinit `y` for a problem created without a `y`" + @assert size(y) == size(cache.y) "reiniting `y` must keep the same size" + copyto!(cache.y, y) end # Update `sigma` inplace if !isnothing(sigma) - @assert wrapper isa NonlinearFunctionWrapper && !isnothing(wrapper.sigma) "cannot reinit `sigma` for a problem created without a `sigma`" - @assert size(sigma) == size(wrapper.sigma) "reiniting `sigma` must keep the same size" - copyto!(wrapper.sigma, sigma) + @assert !isnothing(cache.sigma) "cannot reinit `sigma` for a problem created without a `sigma`" + @assert size(sigma) == size(cache.sigma) "reiniting `sigma` must keep the same size" + copyto!(cache.sigma, sigma) end reinit!(cache.cache; kwargs...) @@ -99,16 +97,23 @@ function CommonSolve.init( problems" @assert prob.u0 !== nothing "Nonlinear curve fitting requires an initial guess (u0)" + x = copyto!(similar(prob.x), prob.x) + y = isnothing(prob.y) ? nothing : copy(prob.y) + sigma = isnothing(prob.sigma) ? nothing : copy(prob.sigma) + return GenericNonlinearCurveFitCache( prob, init( NonlinearLeastSquaresProblem( - __wrap_nonlinear_function(prob.nlfunc, prob.y, prob.sigma), prob.u0, prob.x; + __wrap_nonlinear_function(prob.nlfunc, y, sigma), prob.u0, x; lb = prob.lb, ub = prob.ub ), alg.alg; kwargs... ), + x, + y, + sigma, copy(prob.u0), alg, kwargs @@ -116,29 +121,18 @@ function CommonSolve.init( end function CommonSolve.solve!(cache::GenericNonlinearCurveFitCache) - inner = _get_cache(cache) - x = inner.p sol = solve!(cache.cache) - y = cache.prob.y - sigma = cache.prob.sigma - - wrapped_f = _unwrap_nonlinear_function(inner.prob.f.f) - if wrapped_f isa NonlinearFunctionWrapper - y = wrapped_f.target - sigma = wrapped_f.sigma - end - - # Reconstruct the problem with the current settings. We can't copy + # Reconstruct the problem with the current settings. We can't use # cache.prob because the cache may have been reinit()'d in which case # cache.prob will be out of date and will give wrong results for the stats # functions that use it. prob = CurveFitProblem( - x, - y, - sigma, + cache.x, + cache.y, + cache.sigma, cache.prob.nlfunc, - cache.u0, + copy(cache.u0), cache.prob.lb, cache.prob.ub ) diff --git a/test/nonlinfit_reinit.jl b/test/nonlinfit_reinit.jl index e9283d3..0242cd3 100644 --- a/test/nonlinfit_reinit.jl +++ b/test/nonlinfit_reinit.jl @@ -67,4 +67,16 @@ using NonlinearSolveBase: NonlinearSolveBase cache = CurveFit.init(NonlinearCurveFitProblem(g, [0.5, 0.5, 0.5], x)) @test_throws AssertionError CurveFit.reinit!(cache; y) + + # reinit!() must not modify the user-provided arrays + x = collect(1.0:10.0) + y = g([3.0, 2.0, 0.7], x) + sigma = ones(length(y)) + x_orig, y_orig, sigma_orig = copy(x), copy(y), copy(sigma) + cache = CurveFit.init(NonlinearCurveFitProblem(g, [0.5, 0.5, 0.5], x, y, sigma)) + sol = solve!(cache) + CurveFit.reinit!(cache; x = x .+ 1, y = y .+ 1, sigma = sigma .+ 1) + @test x == x_orig + @test y == y_orig + @test sigma == sigma_orig end