Repository navigation
BUG: Absorb again when AbsorbingLS.fit uses other options - #755
Merged
Merged
Conversation
AbsorbingLS.fit absorbs the variables the first time that it is called and
keeps them. Every later call ignored method, absorb_options, lsmr_options and
use_cache, and used the variables that were absorbed by the first call:
mod = AbsorbingLS(y, x, absorb=cat) # two crossed effects
mod.fit().params # HDFE: x0 0.979689, x1 1.015166
mod.fit(method="lsmr", absorb_options={"maxiter": 1}).params
# x0 0.979689, x1 1.015166
AbsorbingLS(y, x, absorb=cat).fit(
method="lsmr", absorb_options={"maxiter": 1}).params
# x0 0.989950, x1 1.015209
A call that could not be used did not raise either, e.g., method="hdfe" for a
weighted model, or an option that LSMR does not have, where a new model raises.
The variables are now absorbed again if method, absorb_options (also when given
as the deprecated lsmr_options) or use_cache differ from the values that were
used to absorb them. Calls with the same values, for example to change the
covariance estimator, use the absorbed variables as before. The show option is
not compared since it only controls the output. The values are compared when
fit is called, so changing a dict that was passed to a previous call is seen.
This is documented in the Notes of fit.
Absorbing again needs two more changes:
- The state that the first call accumulated is reset: the number of parameters
was increased and the retained columns were selected from the columns that
were left by the previous call. This also fixes a model whose first fit
raised, e.g., because of an invalid option: the degrees of freedom of its
next fit included those of the failed call (df_model 140 instead of 71 in
the tests).
- The cache of absorbed variables (_VARIABLE_CACHE), which is shared by all
models, identified residuals by the regressors and the variable but not by
the LSMR options. With the default use_cache=True, a model that used
maxiter=1 got the residuals of an earlier model that used the default
options whenever the cache held them, and absorbing again would have
returned them too. The options are now part of the key. Passing the default
values explicitly does not change the key.
use_cache=False also wrote to the cache. It stored the residuals of the last
column under an empty key that is never read, which keeps one array in memory
for each model. It does not write any more.
Evidence of the cache problem, without any refit:
clear_cache()
fit(method="lsmr").params # x0 0.979689
fit(method="lsmr", absorb_options={"maxiter": 1}) # x0 0.979689, from the cache
clear_cache()
fit(method="lsmr", absorb_options={"maxiter": 1}) # x0 0.989950
test_options in test_absorbing.py passes without a change. Its second fit asks
for LSMR with other tolerances after an HDFE fit; it did nothing before and now
absorbs the variables with LSMR. The new tests check the results.
Tests (linearmodels/tests/iv/test_absorbing_refit.py, 28 tests): a second fit
with other options is the same as the fit of a new model, in the estimates,
standard errors, df_model, residuals and the absorbed effects, for the HDFE, the
weighted and the continuous variable cases; going back to the defaults; no
change in df_model; reuse of the absorbed variables when only the covariance
estimator, or equal options in another dict, are used (counted); a dict that the
caller changes; lsmr_options; the options of pyhdfe, which are a nested dict;
methods and options that are not valid for a model that has been fit; drop_absorbed
when the absorbed column is not the last one; the cache with different options,
with array valued options and with use_cache=False. 25 of the 28 fail on main.
The three that pass (df_model after a refit) guard against absorbing again
changing the degrees of freedom. Undoing any one part of the change (11 variants
were tried, e.g., not resetting the columns, leaving use_cache or show out of
the comparison, or using the key without the options) makes a test fail.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #755 +/- ##
==========================================
+ Coverage 99.58% 99.59% +0.01%
==========================================
Files 110 111 +1
Lines 20328 20597 +269
Branches 1600 1614 +14
==========================================
+ Hits 20244 20514 +270
Misses 31 31
+ Partials 53 52 -1
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.fit absorbs the variables the first time that it is called and keeps them. Every later call ignored method, absorb_options, lsmr_options and use_cache, and used the variables that were absorbed by the first call:
A call that could not be used did not raise either, e.g., method="hdfe" for a weighted model, or an option that LSMR does not have, where a new model raises.
The variables are now absorbed again if method, absorb_options (also when given as the deprecated lsmr_options) or use_cache differ from the values that were used to absorb them. Calls with the same values, for example to change the covariance estimator, use the absorbed variables as before. The show option is not compared since it only controls the output. The values are compared when fit is called, so changing a dict that was passed to a previous call is seen. This is documented in the Notes of fit.
Absorbing again needs two more changes:
The state that the first call accumulated is reset: the number of parameters was increased and the retained columns were selected from the columns that were left by the previous call. This also fixes a model whose first fit raised, e.g., because of an invalid option: the degrees of freedom of its next fit included those of the failed call (df_model 140 instead of 71 in the tests).
The cache of absorbed variables (_VARIABLE_CACHE), which is shared by all models, identified residuals by the regressors and the variable but not by the LSMR options. With the default use_cache=True, a model that used maxiter=1 got the residuals of an earlier model that used the default options whenever the cache held them, and absorbing again would have returned them too. The options are now part of the key. Passing the default values explicitly does not change the key.
use_cache=False also wrote to the cache. It stored the residuals of the last column under an empty key that is never read, which keeps one array in memory for each model. It does not write any more.
Evidence of the cache problem, without any refit:
test_options in test_absorbing.py passes without a change. Its second fit asks for LSMR with other tolerances after an HDFE fit; it did nothing before and now absorbs the variables with LSMR. The new tests check the results.
Tests (linearmodels/tests/iv/test_absorbing_refit.py, 28 tests): a second fit with other options is the same as the fit of a new model, in the estimates, standard errors, df_model, residuals and the absorbed effects, for the HDFE, the weighted and the continuous variable cases; going back to the defaults; no change in df_model; reuse of the absorbed variables when only the covariance estimator, or equal options in another dict, are used (counted); a dict that the caller changes; lsmr_options; the options of pyhdfe, which are a nested dict; methods and options that are not valid for a model that has been fit; drop_absorbed when the absorbed column is not the last one; the cache with different options, with array valued options and with use_cache=False. 25 of the 28 fail on main. The three that pass (df_model after a refit) guard against absorbing again changing the degrees of freedom. Undoing any one part of the change (11 variants were tried, e.g., not resetting the columns, leaving use_cache or show out of the comparison, or using the key without the options) makes a test fail.