Synchronize p and coefficients

[cbrnr]

As discussed in #177, we should synchronize p and the coefficient matrices. Currently, the user can set p and coef independently, and since p is implicitly defined by the shape of coef, these two variables can get out of sync.

[mbillingr]

I guess it's best if we make p a property that reads its value from the shape of coef.

This leaves the question open how to deal with fitting, where we do not have coefficients yet but need to know the model order to obtain the coefficients. An option could be to initialize the model order to a shape of (0, 0, p) or (p, 0, 0). I'm not sure if this is a nice approach. An alternative could use a private attribute such as _p that contains the desired model order when no coefficients are available.

[cbrnr]

I like the idea to initialize the coefficients - did we decide to use shape (0, 0, p) though? So if p is a property and we set it to a specific value, we would create the corresponding coefficients array, right? What if we already have a specific p and coef and we change either of these - do we overwrite both variables then?

[mbillingr]

I don't know yet on what shape we decided, but (0, 0, p) seems to be a good candidate.

Changing coef would not be a problem because the property p would directly access the shape of the coefficients.

Setting p is a different matter. Option 1: reset the coefficients to (0, 0, p) when pchanges. Option 2: raise an exception when the user tries to set p on a model that has coefficients.

[cbrnr]

OK, (0, 0, p) it is.

This all seems very complicated. Are we doing something wrong? What if we had an API like sklearn? I.e., we initialize with __init__, but set arguments important for fitting in fit? Would that solve the issue of keeping coef and p in sync?

[mbillingr]

fit() in scikit-learn rarely sets any parameters other than the data.

Our 'problem' is that p can be both, a parameter or an optimized (fitted) quantity. This makes the situation somewhat complicated.

We could also make p a read-only property that can only be set in __init__.

[cbrnr]

Actually, fit sets a lot of computed values that all end with an underscore, e.g. coef_, intercept_ etc. But yes, our problem is that p is a parameter and a fitted value. We could call the latter p_, but would that solve the problem of inconsistent values? Which value would other functions take - p or p_?

[mbillingr]

Let's try to consider a few use cases.

a) We have data and want to ft a model with given p.
b) We have data and want to estimate p along with the model.
c) We want to specify model coefficients.

In a) p is a parameter, in b) it is a fitted value, and in c) it is actually in the way because it should be consistent with the coefficients...

Do you think the following API could work?

v = VAR(p=3)
print(v.p)  # 3
v.p = 4  # shall cause an error, p cannot be changed.
v.coef_ = some_array  # causes an error if the shape is not compatible with p (or not 3D)

v = VAR(p='auto')
print(v.p)  # auto
v.fit(data)
print(v.p)  # e.g. 5
v.coef_ = some_array # never causes an error (unless shape is not 3D)
print(v.p)  # e.g. 7 ... whatever the p in some_array was

The implementation could look like this (dummy code):

class VAR:
    def __ init__(p):
        self._p = p
        self._coef = None

    @property
    def p(self):
        return self._coef.shape[-1] if self._coef else p

    @property
    def coef_(self):
        return self._coef

    @coef_.setter
    def coef_(self, value):
        assert value.shape is compatible with "MxMx?"
        if self._p != 'auto':
            assert value.shape[-1] == self.p
        self._coef = value

    def fit(self, data):
        p_to_fit = estimate_p(data) if p == 'auto' else self.p_
        self._coef = estimate_var(data, p_to_fit)

What do you think? Did I miss anything?