madeleineudell / LowRankModels.jl

LowRankModels.jl is a julia package for modeling and fitting generalized low rank models.
Other
190 stars 65 forks source link

Extend Impute.jl interface? #94

Open nickrobinson251 opened 5 years ago

nickrobinson251 commented 5 years ago

I wonder if rather than this package defining impute and impute_missing it would be instead be worth implementing the interface provided by Impute.jl?

One possible way could be to define a LowRank <: Imputor and extend impute(A, imp::LowRank) along the lines of

struct LowRank <: Impute.Imputor
    losses::Vector{Loss}     # Vector of loss functions
    rx::Vector{Regularizer}  # Vector of regularizers to be applied to each column of X
    ry::Vector{Regularizer}  # Vector of regularizers to be applied to each column of Y
    k::Int                   # Desired rank
end

function Impute.impute!(A, imp::LowRank)
    # fit glmr, to get  Â = X Y' ≈ A
    # If A[i, j] is `missing`s replace it with the value Â[i, j]
end

function Impute.impute(A, imp::LowRank)
    # fit glmr, to get  Â = X Y' ≈ A
    # return Â
end

Here are examples of current imputors :)

madeleineudell commented 5 years ago

This is a great idea. I wouldn't do this instead of but in addition to the methods defined internally, in a separate file. (It's better not to be too dependent on dependencies.)

I'd welcome this as a PR; it would be an easy project for someone looking to contribute to this package.

rofinn commented 5 years ago

✋ I can take a look at adding this. I'd like to work on getting the CI passing first.