ENH: Re-use effects if model fit with QR #506

Merged
merged 1 commit into from Nov 14, 2012

Conversation

Projects
None yet
2 participants
Owner

jseabold commented Oct 5, 2012

Just something I remembered. Reuse effects if we have them. Probably not super important, but it's nice to have. Might consider changing default method to QR in linear model fit too.

@@ -106,8 +106,10 @@ def anova1_lm_single(model, endog, exog, nobs, design_info, table, n_rows, test,
Use of this function is discouraged. Use anova_lm instead.
"""
#maybe we should rethink using pinv > qr in OLS/linear models?
- q,r = np.linalg.qr(exog)
- effects = np.dot(q.T, endog)
+ effects = getattr(model, 'effects', None)
@josef-pkt

josef-pkt Oct 5, 2012

Owner

can you check for exog_Q instead?

@jseabold

jseabold Oct 5, 2012

Owner

We need to compute the effects, so I went ahead and attached them during fit as R does. If I check for exog_Q, we still need the effects.

- beta = np.linalg.solve(R,np.dot(Q.T,endog))
+ # used in ANOVA
+ self.effects = effects = np.dot(Q.T, endog)
+ beta = np.linalg.solve(R, effects)
@josef-pkt

josef-pkt Oct 5, 2012

Owner

maybe fine for now, however I thought of removing this calculation from the fit method.
Looks like we can calculate this in a way that uses fewer calculations. (But I haven't checked yet what we really need later on.)

Owner

josef-pkt commented Nov 14, 2012

looks ok to merge.
I assume tests cover this

Owner

jseabold commented Nov 14, 2012

Yeah, used in ANOVA. Will rebase and green button.

Owner

jseabold commented Nov 14, 2012

Still an open issue if we want to switch to QR by default for linear models. Might be other places we could take advantage of attaching this information.

jseabold added a commit that referenced this pull request Nov 14, 2012

Merge pull request #506 from jseabold/reuse-effects
ENH: Re-use effects if model fit with QR

@jseabold jseabold merged commit 48fc384 into statsmodels:master Nov 14, 2012

1 check was pending

default The Travis build is in progress
Details

PierreBdR pushed a commit to PierreBdR/statsmodels that referenced this pull request Sep 2, 2014

Merge pull request #506 from jseabold/reuse-effects
ENH: Re-use effects if model fit with QR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment