SciML / DiffEqParamEstim.jl

Easy scientific machine learning (SciML) parameter estimation with pre-built loss functions
https://docs.sciml.ai/DiffEqParamEstim/stable/
Other
60 stars 34 forks source link

no argument checks and @inbounds is problematic #140

Open ValentinKaisermayer opened 4 years ago

ValentinKaisermayer commented 4 years ago

Consider the following example: https://diffeq.sciml.ai/stable/analysis/parameter_estimation/#Optimization-Based-ODE-Inference-Examples If one makes an L2Loss there is nothing that stops one from doing:

using DiffEqParamEstim, DifferentialEquations, Plots, RecursiveArrayTools

function f(du, u, p, t)
    du[1] = dx = p[1] * u[1] - u[1] * u[2]
    du[2] = dy = -3 * u[2] + u[1] * u[2]
end

u0 = [1.0; 1.0]
tspan = (0.0, 10.0)
p = [1.5]
prob = ODEProblem(f, u0, tspan, p)

sol = solve(prob, Tsit5())
t = collect(range(0, stop=10, length=200))
randomized = VectorOfArray([(sol(t[i]) + .01randn(2)) for i in 1:length(t)])

data = copy(convert(Array, randomized)') # this is the problem

model_ode(p_) = ODEProblem(f, u0, tspan, p_)
loss_objective = build_loss_objective(prob, Tsit5(), L2Loss(t, data); prob_generator=(prob, p) -> model_ode(p))

however, this will result in a crash of Julia. since in https://github.com/SciML/DiffEqParamEstim.jl/blob/fc541edd2f6659a7af8a94bda7fc4f382886e973/src/cost_functions.jl#L54 the correct dimension of data is assumed.

ValentinKaisermayer commented 4 years ago

The same is true if one uses a PeriodicCallback. In this case sol.t might not be equal to loss.t.

ChrisRackauckas commented 3 years ago

I missed this email. That is a good point: it should check the bounds and throw an error before proceeding.