"Fixes linear DRIV shape inconsistency"

[ssemov]

Addresses the shape inconsistency in the linear DRIV implementation.

Changes:

• I modified the effect_interval computation in LinearModelFinalInference class. This is where the inference method for LinearDRIV comes from.
• I added the modification function - reshape_outcomewise_effects - to utilities. Given an effects matrix, it reshapes the second dimension to be consistent with d_y[0].

Related [issue](#687).

Testing:

• TestDRIV --> OK
• test_inference --> OK
• test_statsmodels --> OK

[kbattocchi]

Thanks for the contribution. Unfortunately, these changes are causing test failures in econml.tests.test_dml.TestDML.test_ignores_final_intercept, econml.tests.test_discrete_outcome.TestDiscreteOutcome.test_accuracy, and econml.tests.test_discrete_outcome.TestDiscreteOutcome.test_accuracy_iv.

Given that the shapes are as expected for other estimators (like LinearDML) using the same inference class, I think the best place to address the problem is probably somewhere in _dr.py, although maybe there is a way to address it in the inference class that doesn't break other classes that use it.

[ssemov]

Thanks, @kbattocchi! yes, I think the fix I have touches too many things. I'll look into something more local in _dr.py.

[ssemov]

@kbattocchi I made the change more local and fixed an edge case with the discrete outcome. The new version passes the tests I was failing before.

[ssemov]

Oh yes, that's a good point. I can drop the effect_interval method. The change is in effect_inference.

…

On Wed, Oct 2, 2024 at 11:13 AM Keith Battocchi ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In econml/inference/_inference.py <#920 (comment)>: > + def effect_interval(self, X, *, T0, T1, alpha=0.05): + return self.effect_inference(X, T0=T0, T1=T1).conf_int(alpha=alpha) Isn't this identical to what the base class is already doing? — Reply to this email directly, view it on GitHub <#920 (review)>,

[ssemov]

@kbattocchi Removed the redundant effect_interval method.

[kbattocchi]

Thanks for trying again, but I don't think StatsModelsInference is the right place for the change - see my comment.

[kbattocchi]

In general, the idea of StatsModelsInference compared to LinearModelFinalInference is just to allow setting the covariance estimation method for inference, not to affect any other aspect of the computation, so I think your previous change to that class would make more sense - perhaps with the additional check for () in your reshape_outcomewise_effects implementation changing the LinearModelFinalInference will no longer break other subclasses?

[kbattocchi]

(And I think you'll find that without fixing the change in LinearModelFinalInference, other DRIV subclasses like SparseLinearDRIV still suffer from the same issue - please consider modifying line 457 of test_treatment_featurization to enable further tests on DRIV subclasses)

[ssemov]

@kbattocchi Thanks, Keith, for the help. It makes sense what you are saying, I will work on a fix.

[ssemov]

Changes:

• Moves the fix to LinearModelFinalInference
• updates TestTreatmentFeaturization not to skip LinearDRIV and SparseLinearDRIV

Tests:

• TestDML.test_ignores_final_intercept
• TestDiscreteOutcome.test_accuracy
• TestDiscreteOutcome.test_accuracy_iv
• TestTreatmentFeaturization

[kbattocchi]

Looks good, thanks!