Get rid of DataFrameStatisticalModel

[nalimilan]

The current approach of wrapping fitted StatisticalModel objects within DataFrameStatisticalModel/DataTableStatisticalModel objects has a major limitation: we can only @delegate functions that we know about, i.e. only those defined in StatsBase. This sounds problematic in the long term, where custom model types defined in packages may want to support new operations.

A solution to this would be to move from the current composition approach (wrapper) to inheritance. StatisticalModel object would be required to have some predefined fields, like coefnames, formula, modelmatrix and modelframe. The fit method from StatsModels would set these fields after fitting the model, so that they can be accessed later both by generic methods and by custom methods defined for a given model type.

This solution would for example allow implementing a generic Wald test function in StatsBase (JuliaStats/StatsBase.jl#300), which would use the coefficient names stored in the object. This is not possible with the current DataFrameStatisticalModel approach, where coefficient names can only be accessed by methods designed for DataFrameStatisticalModel, which therefore cannot live in StatsBase.

[andreasnoack]

The key consideration here is how we generally provide a high and low level API's for regression models such that you can enjoy high level convenience while still having access to all the functionality without overhead when you need to fit millions of models. Hence, we'd like to make sure that fields with names are not baked into that whole setup. This was the original reason that GLM's dependency on DataFrames was removed and DataFrameStatisticalModel introduced here.

[nalimilan]

Yes, optional fields would remain null/nothing by default , e.g. if you don't want names for your coefficients.

[andreasnoack]

I'm skeptical that this is right model. Parameter names are not needed for a low level API and they mainly make sense in connection with some kind of dataframe. In the future, we might also be able to fit models based on e.g. streaming data sources so having prespecified fields for all possible regression models seems overly restrictive. Hence, I'm in favor of something like the existing wrapper approach where high level convenience is added alongside the actual fitting procedures in a wrapper object.

[nalimilan]

Why would coefficient names be specific to data frames? Couldn't you want coefficient names even when fitting directly from a matrix or from a streaming source?

I'm not proposing to add field for "all possible regression models": these fields are quite commonly needed for any model family. OTC, the current wrapper approach isn't extensible as any new function a given model family might want to support would need to be added to StatsBase so that we can use it with @delegate. What do you suggest to fix this?

[kleinschmidt]

What's the advantage of fields over accessor methods? It seems like that's orthogonal to the inheritance issue.

…

-- dave.f.kleinschmidt@gmail.com http://davekleinschmidt.com 413-884-2741

On Sep 13, 2017, at 08:16, Milan Bouchet-Valat ***@***.***> wrote: Why would coefficient names be specific to data frames? Couldn't you want coefficient names even when fitting directly from a matrix or from a streaming source? I'm not proposing to add field for "all possible regression models": these fields are quite commonly needed for any model family. OTC, the current wrapper approach isn't extensible as any new function a given model family might want to support would need to be added to StatsBase so that we can use it with @DeleGate. What do you suggest to fix this? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[nalimilan]

The advantage is that StatModels can set these fields after fitting the model, which is needed since the model has no idea of the names and formulas. We could also define setter methods which StatsModels would use, or standardize keyword arguments to pass to fit. That would allow model implementations to choose the best way to store this information.

Finally, we could have a special struct with the fields listed above, and models would just need to provide a function to get/set that object. That would have the advantage of being easily extensible (e.g. if we need to add a new field) without requiring any adjustment from model types. And when you don't need it, that object would be replaced with null/nothing (no need to nullify multiple fields).

[Nosferican]

I get both sides as I am a strong proponent of accessor methods à la StatsBase.jl, but internally I use the special structs heavily. In practice, I like the flexibility and quality control I get from defining each of the applicable methods my structs will use from StatsBase.RegressionModel<:StatsBase.StatisticalModel. Internally, I use a special structs for handling my formulas, pre-modeling phase, actual model, regression output, etc. In practice, if you want things to play well with one another you want an accessor method. Hardcoded structs like DataFrameStatisticalModel usually more pain than ease.

The example you cite about Wald tests for models that have coefficients and a variance covariance matrix is very relevant. However, the way to address it is through a robust API. Methods like coefnames, vcov, coef, dof, dof_residual should be defined at the abstract level of StatsBase.RegressionModel or StatsBase.StatisticalModel. Then it is very easy to play well with others... For a nightmare scenario, CovarianceMatrices.jl is hardcoded to work pretty much only with GLM.jl. Hardcoded structs are also more prone to errors... People might assume that the degrees of freedom are the length of the model and not take into account fixed effects for example. Those scenarios are best handled by accessor methods.

As an aside, usually fit! is a StatsBase.jl associated term. Implementing an advanced fit! at StatsModels.jl would be lowering the level which I would oppose.

tltr: +1 for inheritance.

[kleinschmidt]

It seems really hard to me to avoid some kind of wrapper and still be able to support the high and low level APIs together (or, rather, to get high-level table+formula support for free if you have low-level matrix/vector support). You need some way to keep the formulae together with the fitted model in order to do things like predict from un-observed data.

[kleinschmidt]

Which isn't to say that that wrapper can't inherit from an abstract regression/statistical model type, but that's what the current setup does, right?

[Nosferican]

For example, for instrumental variables the predictions are not modelmatrix(newmodel) * coef(model) where newmodel is derived from newdata, coefnames, Formula, constrasts (coefnames if the linearly dependent variables are dropped or verify NaN in coef if those are kept). The current wrapper allows for one ModelFrame when for example my models use more than four.
For example,

function dof(obj::MyModel)
    mm = modelmatrix(obj)
    output = trace(mm * inv(cholfact!(mm.'mm)) * mm.') - ("(Intercept)" ∈ coefnames(obj))
end

might be a sensible default value, but if there are fixed effects it would be wrong. For example, r2 is defined automatically, but is not defined for instrumental models regardless if they are linear.

I believe the high level API should be left to the packages. However, StatsModels can provide tools to make it easier in a less restrictive manner. For example, maybe having another module or expand StatsBase to allow for a more fine grained abstraction. For example,

instrumental(obj::RegressionModel) = false

and use it for r2.

[nalimilan]

@Nosferican The problem is not with overriding methods, that's perfectly OK and we would never define a default dof fallback as you show above as it's indeed invalid for many models. The issue is about functions which are not part of the basic API defined in StatsBase. For example, it doesn't make a lot of sense to define instrumental in StatsBase, since it does not apply to most models: we don't want to define a new function for each feature a model family would want to support.

Which isn't to say that that wrapper can't inherit from an abstract regression/statistical model type, but that's what the current setup does, right?

@kleinschmidt Yes, but then if a package wants to define e.g. a hazard function for survival models, it needs to define a method for DataFrameRegressionModel which delegates the call to the wrapped model. That's not ideal (and that clearly means all modeling packages need to depend on StatsModels).

It seems really hard to me to avoid some kind of wrapper and still be able to support the high and low level APIs together (or, rather, to get high-level table+formula support for free if you have low-level matrix/vector support). You need some way to keep the formulae together with the fitted model in order to do things like predict from un-observed data.

Why would it be a problem? That's really just composition vs. inheritance. StatisticalModel objects can be required to have a field (or several) where StatsModels functions will store the data it needs (as I proposed above).