perf(within-transform): drop the redundant full-frame copy in within_transform

[igerber]

Summary

• within_transform drops its redundant defensive full-frame copy. The two-way fixed-effects demeaning helper — shared by TwoWayFixedEffects, SunAbraham, WooldridgeDiD, and BaconDecomposition — copied the entire input frame before pd.concat-ing the demeaned columns onto it. But the demean is read-only and concat does not mutate its inputs, so that copy was pure overhead. It now computes the demeaned columns first and attaches them as a single consolidated block via pd.concat with no pre-copy (under pandas copy-on-write the original columns are shared, not copied).
• ~8% peak-RSS reduction for a wide TwoWayFixedEffects(vcov_type="hc1") fit (964 → 886 MB at 400k units × 6 covariates); the win scales with panel width.
• inplace/suffix are decoupled (inplace = attach in place vs concat; suffix = column naming), matching demean_by_group. The non-inplace path drops a colliding target name before the concat so suffix="" / re-demeaning a frame that already carries the suffix overwrite cleanly (one label, no duplicate).
• PR-C of the memory-scaling work (B1 perf(callaway-santanna): chunk multiplier-bootstrap weight generation to bound peak memory #561 + B2 perf(efficient-did, had): chunk multiplier-bootstrap weight generation to bound peak memory #563 merged). The separate ImputationDiD / TwoStageDiD _iterative_demean vectorization is deferred (TODO.md) — its np.bincount form is non-bit-identical on pandas 3.0 (~5.8e-11, the same order as the demean's tol=1e-10) and it is analytical-path-only.

Methodology references

• Method name(s): two-way within (fixed-effects) transformation (FWL) underlying TWFE / SunAbraham / WooldridgeDiD; Goodman-Bacon decomposition cells.
• Paper / source link(s): docs/methodology/REGISTRY.md TWFE within-transform (FWL) section. Frame-assembly-only change; the demeaning arithmetic and weighted alternating-projection warning path are untouched.
• Any intentional deviations from the source (and why): None — bit-identical.

Validation

• Tests added/updated: tests/test_within_transform.py (8 tests — the inplace/suffix modes, overwrite-without-duplicate on both the inplace and concat paths, weighted parity, and the no-PerformanceWarning many-column path).
• Backtest / simulation / notebook evidence: N/A. Verified on both backends; bit-identity proven at atol=0 for TWFE (including the survey replicate-weight path), SunAbraham, Wooldridge, and Bacon via a pre/post estimate capture; full TWFE / SunAbraham / Wooldridge / Bacon / survey / Conley regression green (879 python + 383 rust). Peak RSS 964 → 886 MB on a wide TWFE fit.

Security / privacy

• Confirm no secrets/PII in this PR: Yes

---

🤖 Generated with Claude Code

[github-actions]

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

• Affected methods: TwoWayFixedEffects, SunAbraham, WooldridgeDiD OLS, and BaconDecomposition through shared within_transform.
• Methodology registry requires FWL-consistent demeaning of outcome and regressors; the PR preserves the arithmetic and only changes result-frame assembly.
• No new SE, variance, CI, p-value, or inline inference logic was added.
• The deferred _iterative_demean optimization is tracked in TODO.md, so it is non-blocking.
• I could not run pytest in this environment: pytest and numpy are not installed.

Methodology

No findings.

Severity: None
Impact: The demeaning formulas remain aligned with the registry: unweighted FWL demeaning and weighted alternating projections are unchanged in diff_diff/utils.py:L2716-L2760, with the weighted non-convergence warning preserved at diff_diff/utils.py:L2744-L2750. This matches the TWFE FWL requirement in docs/methodology/REGISTRY.md:L320-L333 and weighted within-transform note in docs/methodology/REGISTRY.md:L4165-L4172.
Concrete fix: None.

Code Quality

P3 informational: within_transform(inplace=True) now uses suffix and therefore adds suffixed columns by default instead of overwriting source columns unless suffix="" is passed. This is tested and documented in the docstring at diff_diff/utils.py:L2669-L2680, and no internal estimator call site uses inplace=True.
Impact: External utility users relying on the old overwrite behavior may be surprised.
Concrete fix: No blocker; consider a changelog compatibility sentence.

Performance

No findings.

Severity: None
Impact: The non-inplace path now builds a single new_block and appends it with pd.concat, avoiding per-column insertion and the removed defensive full-frame copy at diff_diff/utils.py:L2771-L2786.
Concrete fix: None.

Maintainability

No findings.

Severity: None
Impact: The refactor removes duplicated weighted/unweighted inplace vs non-inplace loops and centralizes target-column attachment.
Concrete fix: None.

Tech Debt

P3 informational: The related ImputationDiD._iterative_demean / TwoStageDiD._iterative_demean vectorization is explicitly tracked in TODO.md:L68 with the bit-identity caveat.
Impact: Deferred performance work is documented; not a correctness blocker for this PR.
Concrete fix: None required for approval.

Security

No findings.

Severity: None
Impact: No secrets, I/O surfaces, subprocess execution, or user-controlled execution paths are introduced.
Concrete fix: None.

Documentation/Tests

P3 informational: The new tests cover non-mutating default behavior, inplace/suffix interactions, collision overwrite semantics, weighted inplace/non-inplace parity, and fragmentation warnings in tests/test_within_transform.py:L40-L122. I could not execute them because the review environment lacks pytest and numpy.
Impact: Test coverage is well targeted, but execution was not verified here.
Concrete fix: None for the PR; run pytest -q tests/test_within_transform.py in CI/local dev.