Skip to content

Copy nonlinear fit input arrays into the cache struct #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JamesWrigley wants to merge 1 commit into
base: master
Choose a base branch
from
+54 −32
Open

Copy nonlinear fit input arrays into the cache struct #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 <pjabardo@gmail.com>, Avik Pal <avikpal@mit.edu> and contributors"]

[deps]
Expand Down
9 changes: 9 additions & 0 deletions docs/src/_changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +12 to +17

@ChrisRackauckas ChrisRackauckas Jul 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably us an AbstractAliasSpecifier with the alias kwarg, i.e. the SciMLBase aliasing interface, for this package to control this.


## [v1.10.1] - 2026-07-01

### Fixed
Expand Down
7 changes: 7 additions & 0 deletions src/common_interface.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
56 changes: 25 additions & 31 deletions src/nonlinfit.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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...)
Expand All @@ -99,46 +97,42 @@ 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
)
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
)
Expand Down
12 changes: 12 additions & 0 deletions test/nonlinfit_reinit.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading