From a45fc146d67233aa9d13fb3b0676e1dc1e6d9f37 Mon Sep 17 00:00:00 2001 From: Douglas Bates Date: Thu, 27 Jun 2024 17:26:04 -0500 Subject: [PATCH] More sophisticated checks in restoreoptsum. (#775) Co-authored-by: Phillip Alday --- .gitignore | 2 + NEWS.md | 5 +++ Project.toml | 2 +- src/MixedModels.jl | 2 + src/serialization.jl | 31 +++++++++++--- test/pls.jl | 100 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 134 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index a4fdd21fb..6ce7c9cff 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ docs/jmd LocalPreferences.toml benchmark.md +lcov.info +coverage/ diff --git a/NEWS.md b/NEWS.md index 728e40fd7..fa0da0825 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +MixedModels v4.25.1 Release Notes +============================== +- Use more sophisticated checks on property names in `restoreoptsum` to allow for optsums saved by pre-v4.25 versions to be used with this version and later. [#775] + MixedModels v4.25 Release Notes ============================== - Add type notations in `pwrss(::LinearMixedModel)` and `logdet(::LinearMixedModel)` to enhance type inference. [#773] @@ -538,3 +542,4 @@ Package dependencies [#769]: https://github.com/JuliaStats/MixedModels.jl/issues/769 [#772]: https://github.com/JuliaStats/MixedModels.jl/issues/772 [#773]: https://github.com/JuliaStats/MixedModels.jl/issues/773 +[#775]: https://github.com/JuliaStats/MixedModels.jl/issues/775 diff --git a/Project.toml b/Project.toml index 9e02e7138..52e937998 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "MixedModels" uuid = "ff71e718-51f3-5ec2-a782-8ffcbfa3c316" author = ["Phillip Alday ", "Douglas Bates ", "Jose Bayoan Santiago Calderon "] -version = "4.25.0" +version = "4.25.1" [deps] Arrow = "69666777-d1a9-59fb-9406-91d4454c9d45" diff --git a/src/MixedModels.jl b/src/MixedModels.jl index 231fdfcc5..27037facc 100644 --- a/src/MixedModels.jl +++ b/src/MixedModels.jl @@ -205,6 +205,7 @@ include("mimeshow.jl") include("serialization.jl") include("profile/profile.jl") +# COV_EXCL_START @setup_workload begin # Putting some things in `setup` can reduce the size of the # precompile file and potentially make loading faster. @@ -227,5 +228,6 @@ include("profile/profile.jl") progress) end end +# COV_EXCL_STOP end # module diff --git a/src/serialization.jl b/src/serialization.jl index 525a0e55c..52ec1dafe 100644 --- a/src/serialization.jl +++ b/src/serialization.jl @@ -10,11 +10,24 @@ function restoreoptsum!( ) where {T} dict = JSON3.read(io) ops = m.optsum - okay = - (setdiff(propertynames(ops), keys(dict)) == [:lowerbd]) && - all(ops.lowerbd .≤ dict.initial) && - all(ops.lowerbd .≤ dict.final) - if !okay + allowed_missing = ( + :lowerbd, # never saved, -Inf not allowed in JSON + :xtol_zero_abs, # added in v4.25.0 + :ftol_zero_abs, # added in v4.25.0 + :sigma, # added in v4.1.0 + :fitlog, # added in v4.1.0 + ) + nmdiff = setdiff( + propertynames(ops), # names in freshly created optsum + union!(Set(keys(dict)), allowed_missing), # names in saved optsum plus those we allow to be missing + ) + if !isempty(nmdiff) + throw(ArgumentError(string("optsum names: ", nmdiff, " not found in io"))) + end + if length(setdiff(allowed_missing, keys(dict))) > 1 # 1 because :lowerbd + @warn "optsum was saved with an older version of MixedModels.jl: consider resaving." + end + if any(ops.lowerbd .> dict.initial) || any(ops.lowerbd .> dict.final) throw(ArgumentError("initial or final parameters in io do not satisfy lowerbd")) end for fld in (:feval, :finitial, :fmin, :ftol_rel, :ftol_abs, :maxfeval, :nAGQ, :REML) @@ -33,12 +46,16 @@ function restoreoptsum!( end ops.optimizer = Symbol(dict.optimizer) ops.returnvalue = Symbol(dict.returnvalue) - # provides compatibility with fits saved before the introduction of fixed sigma + # compatibility with fits saved before the introduction of various extensions + for prop in [:xtol_zero_abs, :ftol_zero_abs] + fallback = getproperty(ops, prop) + setproperty!(ops, prop, get(dict, prop, fallback)) + end ops.sigma = get(dict, :sigma, nothing) fitlog = get(dict, :fitlog, nothing) ops.fitlog = if isnothing(fitlog) # compat with fits saved before fitlog - [(ops.initial, ops.finitial, ops.final, ops.fmin)] + [(ops.initial, ops.finitial), (ops.final, ops.fmin)] else [(convert(Vector{T}, first(entry)), T(last(entry))) for entry in fitlog] end diff --git a/test/pls.jl b/test/pls.jl index 6dd8e9583..5d39e69b9 100644 --- a/test/pls.jl +++ b/test/pls.jl @@ -543,6 +543,106 @@ end @test loglikelihood(fm) ≈ loglikelihood(m) @test bic(fm) ≈ bic(m) @test coef(fm) ≈ coef(m) + + # check restoreoptsum from older versions + m = LinearMixedModel( + @formula(reaction ~ 1 + days + (1 + days | subj)), + MixedModels.dataset(:sleepstudy), + ) + iob = IOBuffer( +""" +{ + "initial":[1.0,0.0,1.0], + "finitial":1784.642296192436, + "ftol_rel":1.0e-12, + "ftol_abs":1.0e-8, + "xtol_rel":0.0, + "xtol_abs":[1.0e-10,1.0e-10,1.0e-10], + "initial_step":[0.75,1.0,0.75], + "maxfeval":-1, + "maxtime":-1.0, + "feval":57, + "final":[0.9292213195402981,0.01816837807519162,0.22264487477788353], + "fmin":1751.9393444646712, + "optimizer":"LN_BOBYQA", + "returnvalue":"FTOL_REACHED", + "nAGQ":1, + "REML":false +} +""" + ) + @test_logs((:warn, + r"optsum was saved with an older version of MixedModels.jl: consider resaving"), + restoreoptsum!(m, seekstart(iob))) + @test loglikelihood(fm) ≈ loglikelihood(m) + @test bic(fm) ≈ bic(m) + @test coef(fm) ≈ coef(m) + iob = IOBuffer( +""" +{ + "initial":[1.0,0.0,1.0], + "finitial":1784.642296192436, + "ftol_rel":1.0e-12, + "xtol_rel":0.0, + "xtol_abs":[1.0e-10,1.0e-10,1.0e-10], + "initial_step":[0.75,1.0,0.75], + "maxfeval":-1, + "maxtime":-1.0, + "feval":57, + "final":[0.9292213195402981,0.01816837807519162,0.22264487477788353], + "fmin":1751.9393444646712, + "optimizer":"LN_BOBYQA", + "returnvalue":"FTOL_REACHED", + "nAGQ":1, + "REML":false, + "sigma":null, + "fitlog":[[[1.0,0.0,1.0],1784.642296192436]] +} +""" + ) + @test_throws(ArgumentError("optsum names: [:ftol_abs] not found in io"), + restoreoptsum!(m, seekstart(iob))) + + iob = IOBuffer( +""" +{ + "initial":[1.0,0.0,1.0], + "finitial":1784.642296192436, + "ftol_rel":1.0e-12, + "ftol_abs":1.0e-8, + "xtol_rel":0.0, + "xtol_abs":[1.0e-10,1.0e-10,1.0e-10], + "initial_step":[0.75,1.0,0.75], + "maxfeval":-1, + "maxtime":-1.0, + "feval":57, + "final":[-0.9292213195402981,0.01816837807519162,0.22264487477788353], + "fmin":1751.9393444646712, + "optimizer":"LN_BOBYQA", + "returnvalue":"FTOL_REACHED", + "nAGQ":1, + "REML":false, + "sigma":null, + "fitlog":[[[1.0,0.0,1.0],1784.642296192436]] +} +""" + ) + @test_throws(ArgumentError("initial or final parameters in io do not satisfy lowerbd"), + restoreoptsum!(m, seekstart(iob))) + + # make sure new fields are correctly restored + mktemp() do path, io + m = deepcopy(last(models(:sleepstudy))) + m.optsum.xtol_zero_abs = 0.5 + m.optsum.ftol_zero_abs = 0.5 + saveoptsum(io, m) + m.optsum.xtol_zero_abs = 1.0 + m.optsum.ftol_zero_abs = 1.0 + restoreoptsum!(m, seekstart(io)) + @test m.optsum.xtol_zero_abs == 0.5 + @test m.optsum.ftol_zero_abs == 0.5 + end + end @testset "profile" begin