JuliaNLSolvers / LsqFit.jl

Simple curve fitting in Julia
Other
313 stars 78 forks source link

UndefRefError when using BigFloats #187

Closed atiyo closed 3 years ago

atiyo commented 3 years ago

Edit: accidentally submitted the issue before I had written much. Sorry!

I'm on LsqFit v0.12.0. I gather from https://github.com/JuliaNLSolvers/LsqFit.jl/pull/168 that optimisation over BigFloats should work, and that there are tests for them. But one of the samples in the comments from that PR (https://github.com/JuliaNLSolvers/LsqFit.jl/pull/168#issuecomment-717378547) kicks up an UndefRefError for me.

p0 = rand(BigFloat,2);
curve_fit((x,p)-> p[1] .* x, rand(2), rand(2), p0);

Gives a stacktrace starting with

julia> curve_fit((x,p)-> p[1] .* x, rand(2), rand(2), p0);
ERROR: UndefRefError: access to undefined reference
Stacktrace:
  [1] getindex
    @ ./array.jl:801 [inlined]
  [2] getindex
    @ ./multidimensional.jl:637 [inlined]
  [3] _broadcast_getindex
    @ ./broadcast.jl:614 [inlined]

Thanks in advance for any help, and also for building out and maintaining this package!

jarlebring commented 3 years ago

I confirm this (and it prevents us from using the package for what we need).

pkofod commented 3 years ago

Should be an easy fix.

pkofod commented 3 years ago

Actually, I already fixed this a while back (that's why I thought the fix would be easy, I already did it)... Can you both make sure your packages are up to date? You should have NLSolversBase v.7.8.0 installed (you can check your manifest file).

jarlebring commented 3 years ago

I think I have the latest version:

(@v1.7) pkg> status NLSolversBase
      Status `~/.julia/environments/v1.7/Project.toml`
  [d41bc354] NLSolversBase v7.8.0

(@v1.7) pkg> status LsqFit
      Status `~/.julia/environments/v1.7/Project.toml`
  [2fda8390] LsqFit v0.12.0
julia> p0 = rand(BigFloat,2);
julia> curve_fit((x,p)-> p[1] .* x, rand(2), rand(2), p0);
ERROR: UndefRefError: access to undefined reference
Stacktrace:
  [1] getindex
    @ ./array.jl:835 [inlined]
....
    @ ./broadcast.jl:957 [inlined]
 [14] copy
    @ ./broadcast.jl:929 [inlined]
 [15] materialize
    @ ./broadcast.jl:904 [inlined]
 [16] alloc_DF(x::Vector{BigFloat}, F::Vector{BigFloat})
    @ NLSolversBase ~/.julia/packages/NLSolversBase/geyh3/src/objective_types/abstract.jl:19
 [17] lmfit(f::LsqFit.var"#18#20"{var"#3#4", Vector{Float64}, Vector{Float64}}, p0::Vector{BigFloat}, wt::Vector{Float64}; autodiff::Symbol, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})

The first relevant stacktrace entry seems to be:

https://github.com/JuliaNLSolvers/NLSolversBase.jl/blob/62d2199f70cce78479658da08d346bbc060fc2f7/src/objective_types/abstract.jl#L19

which seems like an unusual way to preallocate a matrix.

jarlebring commented 3 years ago

If I change https://github.com/JuliaNLSolvers/NLSolversBase.jl/blob/62d2199f70cce78479658da08d346bbc060fc2f7/src/objective_types/abstract.jl#L19 to

# Initialize an n-by-n Jacobian
function alloc_DF(x, F)
    Z = zeros(eltype(F),size(F,1),size(x,1))
    Z .= NaN
end

the error goes away. (The code is bad and not good for a PR but supports that alloc_DF assumes the inputs are defined.)

pkofod commented 3 years ago

Wait, so this is v1.7? What about v1.6?

pkofod commented 3 years ago

In a completely new project on v1.6 I get

(blah) pkg> st
      Status `~/blah/Project.toml`
  [2fda8390] LsqFit v0.12.0 `~/.julia/dev/LsqFit`

julia> using LsqFit
[ Info: Precompiling LsqFit [2fda8390-95c7-5789-9bda-21331edee243]

julia> p0 = rand(BigFloat,2);

julia> julia> curve_fit((x,p)-> p[1] .* x, rand(2), rand(2), p0)
LsqFit.LsqFitResult{Vector{BigFloat}, Vector{BigFloat}, Matrix{BigFloat}, Vector{Float64}}(BigFloat[0.3591711217862231819256501084806840416591168094505813896949315656662442056296654, 0.4423207764883399157511475205837046257194973204427780452202791938969429881646886], BigFloat[-0.3197420366797792081183239198317743889659835777328552091923603126827392314571634, 0.1823742015349326183951461626935366844346994163730671775322039795822999472181143], BigFloat[0.400319913061335075354918444645591080188751220703125 0.0; 0.70184874422156706685882454621605575084686279296875 0.0], true, Float64[])
pkofod commented 3 years ago

which seems like an unusual way to preallocate a matrix.

It's actually not unusual, but you may not recognize the pattern. Your code will not work if the input state is a non-vector abstract array. One example is a matrix, another example is a gpuarray.

atiyo commented 3 years ago

Wait, so this is v1.7? What about v1.6?

I'm on Julia v1.6 and am having this error. Just tried a completely fresh environment with only LsqFit in there:

(@v1.6) pkg> activate .
  Activating environment at `~/Documents/lsq_fit_bigfloat_check/Project.toml`

(lsq_fit_bigfloat_check) pkg> st
      Status `~/Documents/lsq_fit_bigfloat_check/Project.toml`
  [2fda8390] LsqFit v0.12.0

(lsq_fit_bigfloat_check) pkg> ^C

julia> using LsqFit

julia> p0 = rand(BigFloat,2);

julia> curve_fit((x,p)-> p[1] .* x, rand(2), rand(2), p0);
ERROR: UndefRefError: access to undefined reference
Stacktrace:
  [1] getindex
    @ ./array.jl:801 [inlined]
  [2] getindex
    @ ./multidimensional.jl:637 [inlined]
  [3] _broadcast_getindex
    @ ./broadcast.jl:614 [inlined]

I'm also on the right version of NLSolversBase I think. Here's the relevant part of my manifest:

[[NLSolversBase]]
deps = ["DiffResults", "Distributed", "FiniteDiff", "ForwardDiff"]
git-tree-sha1 = "50608f411a1e178e0129eab4110bd56efd08816f"
uuid = "d41bc354-129a-5804-8e4c-c37616107c6c"
version = "7.8.0"
pkofod commented 3 years ago

Right, but you are on v0.12.0. The bugfix release v0.12.1 fixed this. Can you try to ]up?

atiyo commented 3 years ago

Ah sorry, I didn't realise that there was a new release. It is working for me on v0.12.1. Thanks very much for the fix!

pkofod commented 3 years ago

Okay. I think we can close this with v0.12.1 then. Thanks for the input everyone!

jarlebring commented 3 years ago

Thanks for the quick response and action to release a new package! That error message is gone now.