Remove installbeta! and make linear model fit insensitive to beta0 state

andreasnoack commented 1 year ago

When looking at https://github.com/JuliaStats/GLM.jl/issues/534 I noticed that fit!(::LinearModel) depended on the state of beta0 stored in the linear predictor. The reason was that the solution stored in delbeta was transferred to beta0 by the installbeta! method which updated the current value. This is important for GLMs but not LMs. Furthermore, the operations are so simple to do with broadcasting that I think we should rather just do that. (I think installbeta! might predate broadcasting in Julia.)

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 :tada:

Comparison is base (ef8bb98) 90.46% compared to head (305f221) 90.48%.

:exclamation: Current head 305f221 differs from pull request most recent head 4734433. Consider uploading reports for the commit 4734433 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #535 +/- ## ========================================== + Coverage 90.46% 90.48% +0.02% ========================================== Files 8 8 Lines 1133 1125 -8 ========================================== - Hits 1025 1018 -7 + Misses 108 107 -1 ``` | [Impacted Files](https://app.codecov.io/gh/JuliaStats/GLM.jl/pull/535?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaStats) | Coverage Δ | | |---|---|---| | [src/linpred.jl](https://app.codecov.io/gh/JuliaStats/GLM.jl/pull/535?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaStats#diff-c3JjL2xpbnByZWQuamw=) | `98.33% <ø> (-0.06%)` | :arrow_down: | | [src/glmfit.jl](https://app.codecov.io/gh/JuliaStats/GLM.jl/pull/535?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaStats#diff-c3JjL2dsbWZpdC5qbA==) | `81.91% <100.00%> (ø)` | | | [src/lm.jl](https://app.codecov.io/gh/JuliaStats/GLM.jl/pull/535?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaStats#diff-c3JjL2xtLmps) | `94.40% <100.00%> (+0.62%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

andreasnoack commented 1 year ago

To give an example of the issue with the current behavior

julia> X = [ones(10) randn(10)];

julia> y = X*ones(2) + randn(10)*0.1;

julia> myp = GLM.DensePredChol(X, false);

julia> myr = GLM.LmResp(y);

julia> coef(fit!(LinearModel(myr, myp, nothing)))
2-element Vector{Float64}:
 0.966074297502665
 1.0104887004807501

julia> coef(fit!(LinearModel(myr, myp, nothing)))
2-element Vector{Float64}:
 1.93214859500533
 2.0209774009615002

julia> coef(fit!(LinearModel(myr, myp, nothing)))
2-element Vector{Float64}:
 2.898222892507995
 3.03146610144225

julia> coef(fit!(LinearModel(myr, myp, nothing)))
2-element Vector{Float64}:
 3.86429719001066
 4.0419548019230005

JuliaStats / GLM.jl

Remove installbeta! and make linear model fit insensitive to beta0 state #535

Codecov Report