Skip to content

Commit 5f47aae

Browse files
committed
fix(multi): honor storage options and validate explicit vcov data
`FixestMulti` stored the full input frame, the estimation config, and the captured evaluation context on the container unconditionally, so `store_data=False` and `lean=True` stripped every child result while the container kept the very frame those options were asked to release. The container now retains `_data`, `_config`, and `_context` only when the options actually permit it; `_parsed` stays unconditional because the container's formula accessors read it. `FixestMulti.vcov()` gains the `data=` argument that child results already accept, which is the only way to update a data-dependent covariance after `store_data=False`. A forwarded frame must describe one estimation sample shared by every child, and that has to be checked once, before any child is touched: children are updated in a loop, so a mismatch discovered halfway through would leave some results on the new covariance and the rest on the old one. The preflight therefore rejects a row count that does not match every child, and rejects children whose split sample or NA mask differ even when their row counts agree, before the loop starts. `QuantregMulti._clear_attributes` deleted `_within_data` from every child after every fit, not only under the storage options, so a default multi-quantile result had no design or response left and could neither predict nor recompute its covariance. Fit state is now published through `_publish_fit_state`, which also sets `_Y_hat_link` and `_Y_hat_response` alongside `_beta_hat`, `_u_hat`, and `_hessian`, and the unconditional deletion loop is gone; each child's own `_clear_attributes` still honors `store_data=False` and `lean=True`.
1 parent c80e9eb commit 5f47aae

4 files changed

Lines changed: 233 additions & 23 deletions

File tree

docs/changelog.qmd

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,13 @@ We also removed:
219219

220220
- Removed `pf.dtable()`; use `maketables.DTable()` directly instead.
221221

222+
Storage options:
223+
224+
- `store_data=False` and `lean=True` now apply to multiple-estimation
225+
containers; `FixestMulti.vcov()` accepts a common explicit estimation sample;
226+
default multi-quantile results retain the arrays needed for prediction and
227+
covariance updates.
228+
222229
### Bug Fixes
223230

224231
- `lean=True` results keep the formula specification and evaluation context, so

pyfixest/estimation/FixestMulti_.py

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from pyfixest.estimation.models.feols_ import Feols
1414
from pyfixest.estimation.models.fepois_ import Fepois
1515
from pyfixest.estimation.plan_ import ParsedFormula
16+
from pyfixest.utils.dev_utils import DataFrameType, _narwhals_to_pandas
1617

1718

1819
class FixestMulti(TidyColumnAccessors):
@@ -67,10 +68,11 @@ def __init__(
6768
context : Mapping[str, Any]
6869
Captured evaluation scope (from `capture_context`).
6970
"""
70-
self._config = config
7171
self._parsed = parsed
72-
self._data = data
73-
self._context = context
72+
if config.store_data and not config.lean:
73+
self._config = config
74+
self._data = data
75+
self._context = context
7476

7577
self.all_fitted_models: dict[str, Feols | Fepois | Feiv] = {}
7678

@@ -119,7 +121,8 @@ def vcov(
119121
self,
120122
vcov: str | dict[str, str],
121123
vcov_kwargs: dict[str, str | int] | None = None,
122-
):
124+
data: DataFrameType | None = None,
125+
) -> FixestMulti:
123126
"""
124127
Update regression inference "on the fly".
125128
@@ -137,13 +140,54 @@ def vcov(
137140
for CRV1 inference or {"CRV3": "clustervar"} for CRV3 inference.
138141
vcov_kwargs : Optional[dict[str, any]]
139142
Additional keyword arguments for the variance-covariance matrix.
143+
data : DataFrameType, optional
144+
The common, already-filtered estimation sample in its original order.
145+
Required for data-dependent covariance updates when the fitted models
146+
were created with `store_data=False`. Defaults to None.
140147
141148
Returns
142149
-------
143-
An instance of the "Fixest" class with updated inference.
150+
FixestMulti
151+
This result container with updated inference.
144152
"""
153+
data_to_forward = data
154+
if data is not None:
155+
try:
156+
data_to_forward = _narwhals_to_pandas(data)
157+
except TypeError as exc:
158+
raise TypeError(
159+
f"The data set must be a DataFrame type. Received: {type(data)}"
160+
) from exc
161+
162+
models = list(self.all_fitted_models.values())
163+
expected_rows = sorted({model._N_rows for model in models})
164+
received_rows = len(data_to_forward)
165+
if any(n_rows != received_rows for n_rows in expected_rows):
166+
raise ValueError(
167+
"`data` passed to FixestMulti.vcov() must contain the common, "
168+
"already-filtered estimation sample in its original order for "
169+
"every child model; expected child row counts "
170+
f"{expected_rows}, received {received_rows}. Fetch each child "
171+
"model and call vcov(..., data=...) separately when estimation "
172+
"samples differ."
173+
)
174+
175+
reference = models[0] if models else None
176+
if reference is not None and any(
177+
model._sample_split_var != reference._sample_split_var
178+
or model._sample_split_value != reference._sample_split_value
179+
or model._na_index != reference._na_index
180+
for model in models[1:]
181+
):
182+
raise ValueError(
183+
"`data` cannot be forwarded by FixestMulti.vcov() because its "
184+
"child models use different estimation samples. Fetch each "
185+
"child model and call vcov(..., data=...) with that model's "
186+
"already-filtered estimation sample instead."
187+
)
188+
145189
for fxst in self.all_fitted_models.values():
146-
fxst.vcov(vcov=vcov, vcov_kwargs=vcov_kwargs)
190+
fxst.vcov(vcov=vcov, vcov_kwargs=vcov_kwargs, data=data_to_forward)
147191
return self
148192

149193
def tidy(self) -> pd.DataFrame:

pyfixest/estimation/quantreg/QuantregMulti.py

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,11 @@ def get_fit(self) -> dict[float, Quantreg]:
111111
fit_kwargs["rng"] = rng
112112
beta_hat = self.all_quantregs[q[q_median_idx]]._fit(**fit_kwargs)[0]
113113

114-
self.all_quantregs[q[q_median_idx]]._beta_hat = beta_hat
115-
self.all_quantregs[q[q_median_idx]]._u_hat = (
116-
Y.flatten() - (X @ beta_hat).flatten()
114+
self._publish_fit_state(
115+
quantreg=self.all_quantregs[q[q_median_idx]],
116+
beta_hat=beta_hat,
117+
hessian=hessian,
117118
)
118-
self.all_quantregs[q[q_median_idx]]._hessian = hessian
119119

120120
def _direction_helper(i, direction):
121121
if direction == "left":
@@ -138,9 +138,11 @@ def _cfm1_fun(i, direction):
138138
beta_hat = self.all_quantregs[q[i]].fit_qreg_pfn(
139139
X=X, Y=Y, q=q[i], beta_init=beta_hat_prev, eta=0.5
140140
)[0]
141-
self.all_quantregs[q[i]]._beta_hat = beta_hat
142-
self.all_quantregs[q[i]]._u_hat = Y.flatten() - (X @ beta_hat).flatten()
143-
self.all_quantregs[q[i]]._hessian = hessian
141+
self._publish_fit_state(
142+
quantreg=self.all_quantregs[q[i]],
143+
beta_hat=beta_hat,
144+
hessian=hessian,
145+
)
144146

145147
for i in range(q_median_idx - 1, -1, -1):
146148
_cfm1_fun(i, "left")
@@ -164,12 +166,11 @@ def _cfm2_fun(i, direction):
164166
M = X.T @ (q[i] - (u_hat_prev < 0))[:, None]
165167
beta_new = beta_hat_prev + np.linalg.solve(J, M).flatten()
166168

167-
self.all_quantregs[q[i]]._beta_hat = beta_new
168-
self.all_quantregs[q[i]]._u_hat = (
169-
self.all_quantregs[q[i]]._Y.flatten()
170-
- self.all_quantregs[q[i]]._X @ beta_new
169+
self._publish_fit_state(
170+
quantreg=self.all_quantregs[q[i]],
171+
beta_hat=beta_new,
172+
hessian=hessian,
171173
)
172-
self.all_quantregs[q[i]]._hessian = hessian
173174

174175
for i in range(q_median_idx - 1, -1, -1):
175176
_cfm2_fun(i, "left")
@@ -188,6 +189,18 @@ def _cfm2_fun(i, direction):
188189
)
189190
return self.all_quantregs
190191

192+
@staticmethod
193+
def _publish_fit_state(
194+
*, quantreg: Quantreg, beta_hat: np.ndarray, hessian: np.ndarray
195+
) -> None:
196+
"""Publish canonical child state after a multi-quantile solver step."""
197+
y_hat = quantreg._X @ beta_hat
198+
quantreg._beta_hat = beta_hat
199+
quantreg._Y_hat_link = y_hat
200+
quantreg._Y_hat_response = y_hat
201+
quantreg._u_hat = quantreg._Y.flatten() - y_hat
202+
quantreg._hessian = hessian
203+
191204
def vcov(
192205
self,
193206
vcov: str | dict[str, str],
@@ -221,9 +234,4 @@ def _clear_attributes(self) -> None:
221234
"Clear all large non-necessary attributes to free memory."
222235
for quantreg in self.all_quantregs.values():
223236
quantreg._clear_attributes()
224-
225-
# `_X` and `_Y` are read-only views, so their backing state is dropped.
226-
for quantreg in self.all_quantregs.values():
227-
if hasattr(quantreg, "_within_data"):
228-
del quantreg._within_data
229237
gc.collect()

tests/test_estimator_state_lifecycle.py

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,26 @@ def test_lean_iv_rejects_first_stage_diagnostics(
478478
getattr(fit, method)()
479479

480480

481+
@pytest.mark.parametrize("fit_kwargs", [{"store_data": False}, {"lean": True}])
482+
def test_multiple_estimation_respects_storage_options(
483+
lifecycle_data: pd.DataFrame,
484+
fit_kwargs: dict[str, bool],
485+
) -> None:
486+
"""The result container must not retain data cleared from all child fits."""
487+
fit = pf.feols(
488+
"y ~ sw(x, x2) | fe",
489+
data=lifecycle_data,
490+
vcov="iid",
491+
**fit_kwargs,
492+
)
493+
494+
assert isinstance(fit, FixestMulti)
495+
assert not hasattr(fit, "_data")
496+
assert not hasattr(fit, "_config")
497+
assert not hasattr(fit, "_context")
498+
assert all(not hasattr(model, "_data") for model in fit.to_list())
499+
500+
481501
def test_lean_results_predict_new_data_without_fixed_effects(
482502
lifecycle_data: pd.DataFrame,
483503
) -> None:
@@ -556,6 +576,117 @@ def test_store_data_false_vcov_uses_explicit_estimation_sample(
556576
assert not hasattr(stripped, "_data")
557577

558578

579+
def test_fixest_multi_forwards_explicit_vcov_data(
580+
lifecycle_data: pd.DataFrame,
581+
) -> None:
582+
"""Multiple-estimation covariance updates forward an explicit sample."""
583+
stripped = pf.feols(
584+
"y ~ sw(x, x2) | fe",
585+
data=lifecycle_data,
586+
vcov="iid",
587+
store_data=False,
588+
)
589+
expected = pf.feols(
590+
"y ~ sw(x, x2) | fe",
591+
data=lifecycle_data,
592+
vcov={"CRV1": "fe"},
593+
)
594+
595+
stripped.vcov({"CRV1": "fe"}, data=lifecycle_data)
596+
597+
for stripped_model, expected_model in zip(
598+
stripped.to_list(), expected.to_list(), strict=True
599+
):
600+
np.testing.assert_allclose(stripped_model._vcov, expected_model._vcov)
601+
assert not hasattr(stripped_model, "_data")
602+
603+
604+
def test_fixest_multi_vcov_preflights_different_estimation_samples(
605+
lifecycle_data: pd.DataFrame,
606+
) -> None:
607+
"""A common-sample mismatch leaves every child model unchanged."""
608+
data = lifecycle_data.copy()
609+
data.loc[data.index[:3], "x2"] = np.nan
610+
fit = pf.feols(
611+
"y ~ sw(x, x2) | fe",
612+
data=data,
613+
vcov="iid",
614+
store_data=False,
615+
)
616+
children = fit.to_list()
617+
inference_before = [
618+
(child._vcov_type_detail, child._vcov.copy()) for child in children
619+
]
620+
621+
with pytest.raises(
622+
ValueError,
623+
match=r"common, already-filtered estimation sample.*\[21, 24\], received 24",
624+
):
625+
fit.vcov({"CRV1": "fe"}, data=data)
626+
627+
for child, (vcov_type_detail, vcov) in zip(children, inference_before, strict=True):
628+
assert child._vcov_type_detail == vcov_type_detail
629+
np.testing.assert_array_equal(child._vcov, vcov)
630+
631+
632+
def test_fixest_multi_vcov_rejects_equal_size_split_samples(
633+
lifecycle_data: pd.DataFrame,
634+
) -> None:
635+
"""Equal row counts do not make distinct split samples interchangeable."""
636+
data = lifecycle_data.assign(sample=np.tile(["left", "right"], 12))
637+
fit = pf.feols(
638+
"y ~ x | fe",
639+
data=data,
640+
split="sample",
641+
vcov="iid",
642+
store_data=False,
643+
)
644+
children = fit.to_list()
645+
inference_before = [
646+
(child._vcov_type_detail, child._vcov.copy()) for child in children
647+
]
648+
left_sample = data.loc[data["sample"] == "left"]
649+
650+
with pytest.raises(
651+
ValueError,
652+
match=r"child models use different estimation samples.*Fetch each child",
653+
):
654+
fit.vcov({"CRV1": "fe"}, data=left_sample)
655+
656+
for child, (vcov_type_detail, vcov) in zip(children, inference_before, strict=True):
657+
assert child._vcov_type_detail == vcov_type_detail
658+
np.testing.assert_array_equal(child._vcov, vcov)
659+
660+
661+
def test_fixest_multi_vcov_rejects_equal_size_distinct_na_masks(
662+
lifecycle_data: pd.DataFrame,
663+
) -> None:
664+
"""Equal-sized formula expansions must retain the same source rows."""
665+
data = lifecycle_data.copy()
666+
data.loc[data.index[0], "x"] = np.nan
667+
data.loc[data.index[1], "x2"] = np.nan
668+
fit = pf.feols(
669+
"y ~ sw(x, x2) | fe",
670+
data=data,
671+
vcov="iid",
672+
store_data=False,
673+
)
674+
children = fit.to_list()
675+
inference_before = [
676+
(child._vcov_type_detail, child._vcov.copy()) for child in children
677+
]
678+
679+
with pytest.raises(
680+
ValueError,
681+
match=r"child models use different estimation samples.*Fetch each child",
682+
):
683+
fit.vcov({"CRV1": "fe"}, data=data.drop(index=0))
684+
685+
for child, (vcov_type_detail, vcov) in zip(children, inference_before, strict=True):
686+
assert child._vcov_type_detail == vcov_type_detail
687+
np.testing.assert_array_equal(child._vcov, vcov)
688+
689+
559690
def test_vcov_rejects_unfiltered_explicit_data(
560691
lifecycle_data: pd.DataFrame,
561692
) -> None:
@@ -758,3 +889,23 @@ def test_quantreg_lean_discards_solver_arrays() -> None:
758889
solver_arrays = ("_x_final", "_s_final", "_z_final", "_w_final", "_y_final")
759890
assert all(not hasattr(fit, attr) for attr in solver_arrays)
760891
assert np.isfinite(fit.coef()).all()
892+
893+
894+
def test_quantreg_multi_retains_default_post_estimation_state() -> None:
895+
"""Default multi-quantile results support prediction and vcov updates."""
896+
rng = np.random.default_rng(20260901)
897+
x = rng.normal(size=200)
898+
data = pd.DataFrame({"y": 1 + 2 * x + rng.normal(size=200), "x": x})
899+
with pytest.warns(FutureWarning, match="experimental"):
900+
multi = pf.quantreg("y ~ x", data=data, quantile=[0.25, 0.75], vcov="iid")
901+
single = pf.quantreg("y ~ x", data=data, quantile=0.25, vcov="hetero")
902+
903+
multi.vcov("hetero")
904+
905+
for model in multi.to_list():
906+
assert np.isfinite(model.predict()).all()
907+
assert np.isfinite(model.se()).all()
908+
909+
first_quantile = multi.fetch_model(0, print_fml=False)
910+
np.testing.assert_allclose(first_quantile.predict(), single.predict())
911+
np.testing.assert_allclose(first_quantile.se(), single.se())

0 commit comments

Comments
 (0)