JuliaStats / GLM.jl

Generalized linear models in Julia
Other
584 stars 114 forks source link

cholpred + @view results in MethodError #470

Closed behinger closed 2 years ago

behinger commented 2 years ago

This is a pure bugreport+MWE, I dont know enough to propose a fix - maybe this is even on purpose

I had a MethodError when using views and GLM.cholpred (via RobustModels.jl).

MWE:

using GLM
using Random

X = [ones(100) rand(MersenneTwister(2),100)]
ix = .!ismissing.(X)

GLM.cholpred(@view(X[ix,:]))

error

MethodError: no method matching cholpred(::SubArray{Float64, 2, Array{Float64, 3}, Tuple{Vector{CartesianIndex{2}}, Base.Slice{Base.OneTo{Int64}}}, false})

Closest candidates are:

cholpred(!Matched::StridedMatrix{T} where T) at ~/.julia/packages/GLM/gt3bb/src/linpred.jl:117

cholpred(!Matched::StridedMatrix{T} where T, !Matched::Bool) at ~/.julia/packages/GLM/gt3bb/src/linpred.jl:117

cholpred(!Matched::SparseArrays.SparseMatrixCSC) at ~/.julia/packages/GLM/gt3bb/src/linpred.jl:188

...

    top-level scope@[Local: 5](http://localhost:1234/edit?id=2e9ea530-a952-11ec-2d47-196d61a3212a#)[inlined]

This is with GLM 1.6.1 on Julia 1.7.2

palday commented 2 years ago

The issue is the type restriction to StridedMatrix:

https://github.com/JuliaStats/GLM.jl/blob/fa74d1450cd3b97a41c6fb025e106a1a5465254d/src/linpred.jl#L104-L117

But I don't think we actually need that restriction -- the cholesky factorization is called on cross product and then X is promoted to AbstractMatrix anyway.