Skip to content

Conversation

@igerber
Copy link
Owner

@igerber igerber commented Jan 13, 2026

Python Code Quality: Within-Transformation Extraction

Added demean_by_group() function to utils.py for one-way fixed effects demeaning
Added within_transform() function to utils.py for two-way (unit + time) FE transformation
Updated 4 modules to use the shared functions:
    estimators.py (2 locations)
    twfe.py
    sun_abraham.py
    bacon.py
Fixed DataFrame fragmentation warning

Python improvements:
- Extract shared within-transformation logic to utils.py
  - New demean_by_group() for one-way fixed effects demeaning
  - New within_transform() for two-way (unit + time) FE transformation
  - Reduces code duplication across estimators.py, twfe.py, sun_abraham.py, bacon.py
  - Fix DataFrame fragmentation warning by building columns in batch

Rust backend optimizations:
- Use Cholesky factorization for matrix inversion in linalg.rs
  - More efficient for symmetric positive-definite matrices (X'X)
  - Approximately half the operations of general LU decomposition
- Reduce bootstrap allocations in bootstrap.rs
  - Allocate directly into pre-allocated buffer
  - Eliminates intermediate Vec<Vec<f64>> -> flatten -> Array2 pattern
  - Single allocation instead of two

Version bump: 2.0.0 -> 2.0.1
The Rust backend optimizations (Cholesky factorization, reduced allocations)
could not be tested in CI due to missing OpenBLAS library. Reverting these
changes to keep v2.0.1 focused on tested Python improvements only.

The Rust optimizations remain in TODO.md for future implementation when
proper testing infrastructure is available.
igerber pushed a commit that referenced this pull request Jan 13, 2026
Review of the TODO items v2.0.1 PR covering:
- Methodology: Verified numerical equivalence of new utility functions
- Code quality: Well-documented, consistent API design
- Performance: ~5x speedup from pandas groupby vs numpy loops
- Maintainability: Eliminates 4 duplicate implementations

Recommendation: APPROVE
@igerber igerber merged commit d851fc3 into main Jan 13, 2026
4 checks passed
@igerber igerber deleted the claude/todo-items-2.0.1-h6tgS branch January 13, 2026 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants