feat(3.9): variance-conventions inventory + component-aware absorbed-FE rank (D3) - #736
Conversation
Overall Assessment✅ Looks good — no unmitigated P0/P1 findings. Executive Summary
Methodology
The affected methods are Code QualityNo findings. Rank is computed before in-place demeaning, and intercept/no-intercept contracts are explicit. Performance
MaintainabilityNo findings. Centralizing rank computation avoids divergent estimator-specific formulas. Tech DebtNo untracked debt. The N-way exact-rank extension and factorization reuse are both recorded in SecurityNo findings. No secrets, unsafe execution, or sensitive data were introduced. Documentation/TestsNo findings. Tests cover connected, disconnected, hierarchical, zero-weight, NaN-key, saturated, estimator-level, and external Runtime tests could not be executed in the review environment because |
…FE rank (D3) The measured map behind the 3.9 variance-consolidation program, plus the first of its fixes. Inventory (docs/methodology/variance-conventions.md, repo-internal): - Measured matrix of every surface reaching the shared clustered-CR1 denominator (linalg._compute_robust_vcov_numpy, vcov_type="hc1" only - a clustered call in any other family fails its row) and every tail-df convention passed to safe_inference/safe_inference_batch. The table is generated from expected literals in tests/test_variance_conventions.py; the parametrized tests assert those literals against live instrumentation (fast subset by default, full sweep under -m slow) and a byte-equality test keeps the committed doc in sync. - Classifies the defects scheduled for the consolidation PRs - D1 (absorb= vs fixed_effects=: 10.35% SE split on the same model, ratio exactly sqrt((360-2)/(360-66))), D2 (clustered CR1 never counts absorbed FE not nested in the cluster; anti-conservative, up to 5.51% at G=60), D4 (SunAbraham reports residual df per cohort-period cell but normal theory on aggregates) - and the legitimate exceptions L1-L4 (StackedDiD CR1S, LPDiD G-1, influence-function paths, hc2/hc2_bm + survey TSL) with reasons and external anchors. D3 fix (this PR's behavior change): - New diff_diff.utils.absorbed_fe_rank: two-way absorbed-FE df from the connected components of the bipartite level graph (sum(levels) - C, minus 1 when the visible design carries an intercept column), exact per Abowd-Creecy-Kramarz. Levels and edges use positive-weight rows only, per the REGISTRY zero-weight-padding guarantee; NaN group keys raise an actionable ValueError. N>=3 keeps sum(levels-1) with the over-count limitation documented and tracked in TODO.md. - Consumers: DiD(absorb=) and MultiPeriodDiD(absorb=) (intercept form, computed on the pre-transform frame), within-transform TwoWayFixedEffects (intercept form), SunAbraham (no-intercept form - its saturated design has coef_offset=0). Connected independent panels are bit-identical to the old count; disconnected and hierarchical designs (absorb=["state","state_year"]: true rank 29, old count 34) get corrected residual df and non-clustered classical/HC1 SE scaling, with the fail-closed NaN boundary moving consistently in both directions. - External anchor: matches fixest::ssc(K.exact = TRUE) at <=1e-12 on a committed hierarchical R golden (benchmarks/data/fixest_kexact_golden.json, fixest 0.14.2, generator script included); fixest's DEFAULT K.exact=FALSE reproduces the old approximate count, recorded as a labeled deviation from the R default in REGISTRY.md. - demean_by_groups/demean_by_group's second return value is now documented as the raw level count, NOT a valid df adjustment, pointing to absorbed_fe_rank (no callers consume it as df any more; returning the rank from the demeaner instead was rejected because within_transform delegates there and would compute the graph twice per TWFE/SA fit). - REGISTRY.md: the TwoWayFixedEffects absorbed-FE df note is now component-aware, with cross-references from the DiD, MultiPeriodDiD, and SunAbraham sections; both sides of the D1 split and the Wooldridge hc1 mechanism status are documented. doc-deps.yaml registers the inventory doc against its 11 source modules; performance-plan.md records the helper's measured 1.9 ms / 186k-row cost with the codes-reuse optimization tracked as its own TODO row.
3401c81 to
993b2d0
Compare
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good — no unmitigated P0/P1 findings. Executive Summary
Methodology
Propagation and intercept handling are correct in Code QualityNo findings. Performance
MaintainabilityNo findings. Centralizing rank calculation removes estimator-specific df formulas. Tech DebtNo untracked debt. N-way exact rank and factorization reuse are recorded in SecurityNo findings. No secrets, unsafe execution, or sensitive data were introduced. Documentation/TestsNo findings. Tests cover connected, disconnected, hierarchical, zero-weight, NaN-key, saturated, estimator-level, and fixest-parity cases. Runtime tests could not be executed because |
1 participant
Summary
docs/methodology/variance-conventions.md(repo-internal, Sphinx-excluded): the measured inventory of every surface reaching the shared clustered-CR1 denominator and every tail-df convention, with a defect/legitimate classification (D1/D2/D4 scheduled for the 3.9 consolidation PRs; L1-L4 declared exceptions with reasons and external anchors). The table is generated from expected literals intests/test_variance_conventions.py; a byte-equality test keeps the committed doc in sync.tests/test_variance_conventions.py: the audit matrix. Programmatically discovered instrumentation (every module bindingsafe_inference/safe_inference_batch; Rust backend disabled per module) captures each row's CR1kmultiset (vcov_type="hc1"only - a clustered call in any other family fails the row) and tail-df multiset against pinned literals, fast subset by default and the full sweep under-m slow.diff_diff.utils.absorbed_fe_rankcomputes the two-way absorbed-FE df from the connected components of the bipartite level graph (sum(levels) - C, minus 1 when the visible design carries an intercept column), replacing thesum(levels - 1)count that over-counted on disconnected and hierarchical panels (absorb=["state", "state_year"]: true rank 29, old count 34). Consumed byDifferenceInDifferences(absorb=),MultiPeriodDiD(absorb=), within-transformTwoWayFixedEffects(intercept form), andSunAbraham(no-intercept form). Connected independent panels are bit-identical; levels and graph edges use positive-weight rows only per the REGISTRY zero-weight-padding guarantee; NaN group keys raise an actionableValueError;N >= 3keeps the legacy count with the limitation documented and tracked.demean_by_groups/demean_by_groupsecond return value as a raw level count that is NOT a valid df adjustment (no library caller consumes it as df any more), and updates REGISTRY.md: the component-aware TwoWayFixedEffects absorbed-FE df note with cross-references from the DiD/MPD/SunAbraham sections, both sides of the D1 split, the labeled deviation from the fixest default, and the Wooldridge hc1 mechanism status.Methodology references (required if estimator / math changes)
fixest::ssc(K.exact = TRUE)documentation (exact irregular-FE parameter count)fixest::ssc(K.exact = TRUE)rather than fixest's approximate DEFAULT (K.exact = FALSE, which reproduces the old over-count); recorded as a labeled "Note (deviation from R default - irregular FE designs)" in REGISTRY.md with measured numbers, and anchored by a committed R golden (benchmarks/data/fixest_kexact_golden.json, fixest 0.14.2) at <= 1e-12. Pre-existing D1/D2/D4 conventions are inventoried and documented, not changed here (scheduled follow-up PRs).Validation
tests/test_variance_conventions.py(new: audit matrix, absorbed_fe_rank unit + end-to-end arms incl. disconnected/hierarchical/zero-weight/NaN-key/fail-closed-boundary cases, doc byte-equality sync, skip-guarded fixest K.exact parity),tests/test_utils.py(pins the demean count as the raw level count vs the df-valid rank)benchmarks/R/generate_fixest_kexact_golden.R(fixest 0.14.2, deterministic embedded data); classical SE matches theK.exact = TRUEside to all printed digits and discriminates from the R default (>2% apart)Security / privacy