fix(estimation): estimation-state contracts for refits, storage, and unsupported paths - #1501
Closed
s3alfisc wants to merge 10 commits into
Closed
fix(estimation): estimation-state contracts for refits, storage, and unsupported paths#1501s3alfisc wants to merge 10 commits into
s3alfisc wants to merge 10 commits into
Conversation
s3alfisc
force-pushed
the
fix/estimation-state-contracts
branch
from
September 3, 2026 20:57
4e23806 to
b33a5de
Compare
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
A CRV3 request on a gaussian, logit, or probit result was accepted and then refit each leave-one-cluster-out sample through the Poisson API, so the jackknife covariance described a different model than the one reported. Feglm now disables CRV3 support; Fepois keeps the longstanding Poisson jackknife path, which is unchanged numerically and exercised through both feglm(family="poisson") and fepois().
The CRV3 slow jackknife and the slow randomization-inference loop rebuilt each leave-one-cluster-out or resampled sample by calling feols()/fepois() with only `weights` and `weights_type`. Everything else the user chose was silently reset to the API defaults: the small-sample correction, singleton removal, the solver, the demeaner and its tolerances, the collinearity tolerance, `drop_intercept`, the formula evaluation context, and for Poisson the offset, the IRLS tolerance and iteration cap, and the separation checks. The resulting jackknife coefficients therefore came from a different estimator than the one whose covariance was being reported. `_estimation_refit_kwargs()` now carries that contract and both refit paths replay it. A `Preconditioner` handed to `LsmrDemeaner` is the one option that must not be forwarded verbatim: a prebuilt factorization is tied to the fixed-effect design of the full sample, and every leave-one-cluster-out sample has a different one. Its algorithmic variant is kept and the factorization is rebuilt per refit.
For a weighted fit, `fixef()` solved the least-squares problem on the square-root-weighted design and then stored `sqrt(W) * D @ alpha` in `_sumFE`, so the retained fixed-effect contribution -- and therefore `predict()` whenever fixed effects were present -- was on the solver scale rather than in response units. The `lsqr` solve still uses the weighted design; only the retained product switches to the unweighted one. R fixest reports `sumFE` on the response scale, and the new `against_r_core` test compares `_sumFE`, `fixef()`, fitted values, and `predict(newdata=...)` against `fixest` for both analytic and frequency weights.
Computing the effective F statistic refits the first stage with heteroskedasticity-robust errors. As a side effect it also overwrote the outer model's `_vcov_type_detail` with "hetero", so after `IV_Diag()` a later `vcov()` call or a report read a covariance label the main model had never been estimated with, while its stored standard errors still came from the requested type. Only the label is removed. The effective F was and remains computed from the first stage's heteroskedasticity-robust covariance, so the statistic is unchanged and matches the value obtained from a `vcov="hetero"` fit.
`wildboottest()`, `ritest()`, and `decompose()` reached their OLS solver
paths for every estimator that inherits them. On a `Feglm` result they read
`_X`, `_Y`, and `_u_hat`, which hold the IRLS working design, working
response, and working residuals of the final iteration, and treated them as
the OLS design, dependent variable, and residuals. On a `Quantreg` result
the same attributes hold the quantile solver's inputs and check-function
residuals, which carry no least-squares interpretation at all. Both cases
returned bootstrap distributions, randomization p-values, and Gelbach
decompositions that look like valid output but describe a linear model
nobody estimated.
Each path now rejects the estimators it cannot represent: the wild bootstrap
via the `_supports_wildboottest` flag, which `Feglm` now sets to `False`
alongside `Quantreg`; randomization inference via an explicit `_method`
check against `{"feols", "fepois"}`; and `decompose()` via
`_support_decomposition`, which `Quantreg` now sets to `False` next to the
existing `False` on `Feglm`, `Fepois`, and `Feiv`.
`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`.
Records in the architecture guide which post-estimation paths reject non-OLS results, that CRV3 and slow randomization refits replay the estimation contract, and how the storage options now reach multiple-estimation containers. Files the container storage note under bug fixes next to the related first-stage note instead of under the removals list.
The lookup has held array-native DemeanedData entries since the shared demean cache stopped converting to DataFrames; the annotation still said pd.DataFrame. Annotation only, no runtime change.
s3alfisc
force-pushed
the
fix/estimation-state-contracts
branch
2 times, most recently
from
September 3, 2026 21:49
03bcff6 to
e078430
Compare
This was referenced Sep 4, 2026
# Conflicts: # docs/developer/architecture.md # pyfixest/estimation/models/feols_.py
This was referenced Sep 5, 2026
Member
Author
|
Superseded by a nine-layer replacement stack above unchanged #1499, rebuilt from this PR's reviewed final code (head 03f284b). Each layer answers one review question and passes its checks against its immediate parent:
The final layer's tracked-file contents are identical to 03f284b. Closing as superseded; this branch and the review discussion stay in place for reference. |
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
1 participant
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.
Completes the shared estimator-state lifecycle: covariance updates are computed on detached candidates and published only after covariance and inference succeed. Single models and both multi-model containers retain their existing identities and previous results on failure. Gaussian
get_performance()now uses the appropriate shared within-response accessor rather than assuming linear-only state.Preserves the existing raw-scale weighted FE reconstruction, storage cleanup, unsupported-path guards, and faithful CRV3/randomization refits.
update(inplace=True)remains explicitly rejected. Gaussian offsets remain unsupported; explicit covariance data must already match the filtered sample and order.Integrates reviewed #1500 through an additive merge. No reviewed history was rewritten, and #1504 or its descendants were not adjusted.
Local verification: release contract 1,355 passed; lifecycle 65 passed; focused checks 9 passed; Gaussian/R-reference plus offset rejection 2 passed; covariance/HAC primitives 16 passed; live-R HAC 353 passed with 184 non-applicable combinations skipped; Ruff, mypy, and whitespace checks passed. Fresh exact-head CI supplies remaining Python/R/docs evidence. #1500's Ubuntu R preflight hit an unchanged auxiliary quantile-solver convergence failure; same-runtime old/new comparisons pass locally, but that failed CI check remains unresolved. Not merge-ready pending reference/platform evidence and human review.