From e0eee835ae5184b35bc665dae42ee7af19b9e3ed Mon Sep 17 00:00:00 2001 From: Douglas Bates Date: Thu, 27 Jun 2024 12:14:36 -0500 Subject: [PATCH 01/11] More sophisticated checks in restoreoptsum. --- src/serialization.jl | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/serialization.jl b/src/serialization.jl index 525a0e55c..92b60560b 100644 --- a/src/serialization.jl +++ b/src/serialization.jl @@ -10,11 +10,21 @@ 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 + nmdiff = setdiff( + propertynames(ops), # names in freshly created optsum + union!( # names in saved optsum plus those we allow to be missing + Set(keys(dict)), + ( + :lowerbd, # never saved, -Inf not allowed in JSON + :xtol_zero_abs, # added in v4.25.0 + :ftol_zero_abs, # added in v4.25.0 + ) + ) + ) + if !isempty(nmdiff) + throw(ArgumentError("optsum names:", nmdiff, " not found in io")) + 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) From 64a8230c160a465187b4f0acfb44f39be54facb9 Mon Sep 17 00:00:00 2001 From: Douglas Bates Date: Thu, 27 Jun 2024 12:19:55 -0500 Subject: [PATCH 02/11] Bump version and update NEWS.md --- NEWS.md | 5 +++++ Project.toml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 728e40fd7..35d41716a 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. + 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 +[#774]: https://github.com/JuliaStats/MixedModels.jl/issues/774 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" From b8f72455bb056387ab6ee3c69d69e352290037bd Mon Sep 17 00:00:00 2001 From: Douglas Bates Date: Thu, 27 Jun 2024 12:22:06 -0500 Subject: [PATCH 03/11] formatter [ci skip] --- src/serialization.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/serialization.jl b/src/serialization.jl index 92b60560b..8908825cd 100644 --- a/src/serialization.jl +++ b/src/serialization.jl @@ -18,8 +18,8 @@ function restoreoptsum!( :lowerbd, # never saved, -Inf not allowed in JSON :xtol_zero_abs, # added in v4.25.0 :ftol_zero_abs, # added in v4.25.0 - ) - ) + ), + ), ) if !isempty(nmdiff) throw(ArgumentError("optsum names:", nmdiff, " not found in io")) From 50b445e3429afa0d44abbdb4a3cb62d3fbde7300 Mon Sep 17 00:00:00 2001 From: Douglas Bates Date: Thu, 27 Jun 2024 12:44:41 -0500 Subject: [PATCH 04/11] Update NEWS [ci skip] --- NEWS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 35d41716a..fa0da0825 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ 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. +- 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 ============================== @@ -542,4 +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 -[#774]: https://github.com/JuliaStats/MixedModels.jl/issues/774 +[#775]: https://github.com/JuliaStats/MixedModels.jl/issues/775 From 87262fb08655535e29f3f787656e5eeb02b16b58 Mon Sep 17 00:00:00 2001 From: Douglas Bates Date: Thu, 27 Jun 2024 14:51:05 -0500 Subject: [PATCH 05/11] Add tests for restoreoptsum changes and correct error message. --- src/serialization.jl | 2 +- test/pls.jl | 86 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/serialization.jl b/src/serialization.jl index 8908825cd..f136b239c 100644 --- a/src/serialization.jl +++ b/src/serialization.jl @@ -22,7 +22,7 @@ function restoreoptsum!( ), ) if !isempty(nmdiff) - throw(ArgumentError("optsum names:", nmdiff, " not found in io")) + throw(ArgumentError(string("optsum names:", nmdiff, " not found in io"))) end if any(ops.lowerbd .> dict.initial) || any(ops.lowerbd .> dict.final) throw(ArgumentError("initial or final parameters in io do not satisfy lowerbd")) diff --git a/test/pls.jl b/test/pls.jl index 6dd8e9583..d4645b77b 100644 --- a/test/pls.jl +++ b/test/pls.jl @@ -543,8 +543,92 @@ end @test loglikelihood(fm) ≈ loglikelihood(m) @test bic(fm) ≈ bic(m) @test coef(fm) ≈ coef(m) - end + # check restoreoptsum from v4.23.1 + 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, + "sigma":null, + "fitlog":[[[1.0,0.0,1.0],1784.642296192436]] +} +""" + ) + 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 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 restoreoptsum!(m, seekstart(iob)) + end + @testset "profile" begin pr = profile(last(models(:sleepstudy))) tbl = pr.tbl From a0c4da66c6ecd08438439f0d789fe84040d5a030 Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 27 Jun 2024 15:36:26 -0500 Subject: [PATCH 06/11] trim trailing whitespace --- test/pls.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/pls.jl b/test/pls.jl index d4645b77b..50e461735 100644 --- a/test/pls.jl +++ b/test/pls.jl @@ -626,9 +626,9 @@ end } """ ) - @test_throws ArgumentError restoreoptsum!(m, seekstart(iob)) + @test_throws ArgumentError restoreoptsum!(m, seekstart(iob)) end - + @testset "profile" begin pr = profile(last(models(:sleepstudy))) tbl = pr.tbl From f71ed95e54ec4d132d6240440ad0cd42b535bb75 Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 27 Jun 2024 15:54:18 -0500 Subject: [PATCH 07/11] better backwards compat --- src/serialization.jl | 12 +++++++++--- test/pls.jl | 12 ++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/serialization.jl b/src/serialization.jl index f136b239c..14a4d41bf 100644 --- a/src/serialization.jl +++ b/src/serialization.jl @@ -18,11 +18,13 @@ function restoreoptsum!( :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 ), ), ) if !isempty(nmdiff) - throw(ArgumentError(string("optsum names:", nmdiff, " not found in io"))) + throw(ArgumentError(string("optsum names: ", nmdiff, " not found in io"))) end if any(ops.lowerbd .> dict.initial) || any(ops.lowerbd .> dict.final) throw(ArgumentError("initial or final parameters in io do not satisfy lowerbd")) @@ -43,12 +45,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 50e461735..9aa7109de 100644 --- a/test/pls.jl +++ b/test/pls.jl @@ -544,7 +544,7 @@ end @test bic(fm) ≈ bic(m) @test coef(fm) ≈ coef(m) - # check restoreoptsum from v4.23.1 + # check restoreoptsum from older versions m = LinearMixedModel( @formula(reaction ~ 1 + days + (1 + days | subj)), MixedModels.dataset(:sleepstudy), @@ -567,9 +567,7 @@ end "optimizer":"LN_BOBYQA", "returnvalue":"FTOL_REACHED", "nAGQ":1, - "REML":false, - "sigma":null, - "fitlog":[[[1.0,0.0,1.0],1784.642296192436]] + "REML":false } """ ) @@ -600,7 +598,8 @@ end } """ ) - @test_throws ArgumentError restoreoptsum!(m, seekstart(iob)) + @test_throws(ArgumentError("optsum names: [:ftol_abs] not found in io"), + restoreoptsum!(m, seekstart(iob))) iob = IOBuffer( """ @@ -626,7 +625,8 @@ end } """ ) - @test_throws ArgumentError restoreoptsum!(m, seekstart(iob)) + @test_throws(ArgumentError("initial or final parameters in io do not satisfy lowerbd"), + restoreoptsum!(m, seekstart(iob))) end @testset "profile" begin From 43f4a7283080fd08995bb50ed8c5c44c958f21a2 Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 27 Jun 2024 16:03:23 -0500 Subject: [PATCH 08/11] test loading of new fields --- test/pls.jl | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/pls.jl b/test/pls.jl index 9aa7109de..3c96fc529 100644 --- a/test/pls.jl +++ b/test/pls.jl @@ -627,6 +627,20 @@ end ) @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 = refit!(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 From 6774001337a86aaec48709dba32dab8f430148d7 Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 27 Jun 2024 16:16:10 -0500 Subject: [PATCH 09/11] issue warning when loading legacy fits --- src/serialization.jl | 21 +++++++++++---------- test/pls.jl | 6 ++++-- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/serialization.jl b/src/serialization.jl index 14a4d41bf..52ec1dafe 100644 --- a/src/serialization.jl +++ b/src/serialization.jl @@ -10,22 +10,23 @@ function restoreoptsum!( ) where {T} dict = JSON3.read(io) ops = m.optsum + 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!( # names in saved optsum plus those we allow to be missing - Set(keys(dict)), - ( - :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 - ), - ), + 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 diff --git a/test/pls.jl b/test/pls.jl index 3c96fc529..5d39e69b9 100644 --- a/test/pls.jl +++ b/test/pls.jl @@ -571,7 +571,9 @@ end } """ ) - restoreoptsum!(m, seekstart(iob)) + @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) @@ -630,7 +632,7 @@ end # make sure new fields are correctly restored mktemp() do path, io - m = refit!(deepcopy(last(models(:sleepstudy)))) + m = deepcopy(last(models(:sleepstudy))) m.optsum.xtol_zero_abs = 0.5 m.optsum.ftol_zero_abs = 0.5 saveoptsum(io, m) From de113d70f02e4fcefa448bf33c87b599bce9984c Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 27 Jun 2024 16:33:13 -0500 Subject: [PATCH 10/11] exclude precompile from coverage --- src/MixedModels.jl | 2 ++ 1 file changed, 2 insertions(+) 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 From a2b08573055eaa24f4a165576c1c38d15e29f0b8 Mon Sep 17 00:00:00 2001 From: Phillip Alday Date: Thu, 27 Jun 2024 16:34:06 -0500 Subject: [PATCH 11/11] ignore coverage --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) 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/