Repository navigation
BUG: Fix AbsorbingLS models that drop observations with missing values - #766
Merged
Merged
Conversation
AbsorbingLS drops observations that have a missing value in dependent, exog,
absorb or interactions, but a model that dropped any row could never be fit.
A single missing value in any input made fit raise
AbsorbingLS(y, x_with_one_nan, absorb=cat).fit()
ValueError: operands could not be broadcast together with shapes (120,1) (119,1)
or, with weights, "matmul: dimension mismatch". Missing values are dropped by
default, so this affected every user with a NaN in the data. It has been the
case since AbsorbingLS was added (4.13, #204) and is in 7.0. The only test of
the dropping, test_drop_missing, constructs the model and never calls fit.
Causes, all in the dropping of rows:
- _drop_missing never dropped the weights, so the weights kept the number of
rows before the drop. This was the broadcast and matmul error.
- NaN weights were not treated as missing.
- _nobs kept the number of rows before the drop. It sets the residual degrees
of freedom of the F-statistic when debiased=True, so the p-value was wrong
for any model that did drop rows.
- With absorb=None (only interactions), an empty Interaction stored the same
IVData object as its categorical and its continuous data. Dropping rows
dropped them twice and raised "IndexError: Boolean index has wrong length".
- Interaction.drop works in place, so an Interaction passed by the caller was
shortened by the model: inter.cat had fewer rows than inter.nobs and the next
model that used the instance failed or paired the wrong rows.
- If every row was missing, the error came from deep inside the model instead
of the "All observations contain missing data" message of the IV models.
Fix: drop the weights with the other inputs, treat NaN weights as missing,
refresh _nobs, give an empty Interaction two IVData objects, drop rows from
copies of the Interaction instances, and raise the same ValueError as the IV
estimators when all rows are missing.
The weights are normalized to have mean one when the model is created, which is
before the rows are dropped. They are rescaled after the drop, so that they
have mean one in the estimation sample. Params, covariances, R2 and F do not
depend on the scale of the weights, but resid_ss, total_ss, s2, wresids and
absorbed_effects do. With the rescaling the results are identical to the
results from dropping the rows by hand, in every output.
clusters: when cov_type is "clustered", clusters is checked against the number
of observations that remain after the drop, which is how the IV estimators
behave. Clusters with one row for each original observation raise the "clusters
has the wrong nobs" ValueError. This is documented in the Notes of fit. (The
text of that error refers to the model property notnull, which AbsorbingLS does
not have.)
Tests (linearmodels/tests/iv/test_absorbing_missing.py, 190 tests): the results
of a model with missing values are compared in all outputs (params, standard
errors, covariance, nobs, df, R2, F statistic and p-value, resid_ss, total_ss,
s2, residuals, weighted residuals, fitted and absorbed effects, confidence
intervals) with the results of the model fit to manually dropped rows, for
- NaN in dependent, exog, absorb (categorical and continuous), weights and
interaction (categorical and continuous) data;
- absorb given as a DataFrame of categoricals (HDFE), a DataFrame with a
continuous column (LSMR), an Interaction and None;
- with and without weights and interactions, debiased True and False;
- the unadjusted, robust, kernel and clustered covariances;
- NumPy inputs, a category that disappears when its only row is dropped, and
drop_absorbed.
One test compares with the IV2SLS estimator with dummy variables and weights,
fit to the dropped data. The tests fail on main (190 of 190) and pass here.
Removing any one part of the fix makes some of them fail.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #766 +/- ##
========================================
Coverage 99.58% 99.59%
========================================
Files 110 111 +1
Lines 20328 20631 +303
Branches 1600 1632 +32
========================================
+ Hits 20244 20547 +303
Misses 31 31
Partials 53 53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AbsorbingLS drops observations that have a missing value in dependent, exog, absorb or interactions, but a model that dropped any row could never be fit. A single missing value in any input made fit raise
or, with weights, "matmul: dimension mismatch". Missing values are dropped by default, so this affected every user with a NaN in the data. It has been the case since AbsorbingLS was added (4.13, #204) and is in 7.0. The only test of the dropping, test_drop_missing, constructs the model and never calls fit.
Causes, all in the dropping of rows:
Fix: drop the weights with the other inputs, treat NaN weights as missing, refresh _nobs, give an empty Interaction two IVData objects, drop rows from copies of the Interaction instances, and raise the same ValueError as the IV estimators when all rows are missing.
The weights are normalized to have mean one when the model is created, which is before the rows are dropped. They are rescaled after the drop, so that they have mean one in the estimation sample. Params, covariances, R2 and F do not depend on the scale of the weights, but resid_ss, total_ss, s2, wresids and absorbed_effects do. With the rescaling the results are identical to the results from dropping the rows by hand, in every output.
clusters: when cov_type is "clustered", clusters is checked against the number of observations that remain after the drop, which is how the IV estimators behave. Clusters with one row for each original observation raise the "clusters has the wrong nobs" ValueError. This is documented in the Notes of fit. (The text of that error refers to the model property notnull, which AbsorbingLS does not have.)
Tests (linearmodels/tests/iv/test_absorbing_missing.py, 190 tests): the results of a model with missing values are compared in all outputs (params, standard errors, covariance, nobs, df, R2, F statistic and p-value, resid_ss, total_ss, s2, residuals, weighted residuals, fitted and absorbed effects, confidence intervals) with the results of the model fit to manually dropped rows, for