Skip to content

Commit 34f377a

Browse files
committed
adds plan + rules
1 parent 170391c commit 34f377a

File tree

2 files changed

+94
-0
lines changed

2 files changed

+94
-0
lines changed
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
<!-- 5e7c976a-5d57-4559-9eec-e537dca963a1 9818a1cd-a593-4f12-a9f7-6caba18c5fcb -->
2+
# Add Fixed Effects Support to feglm (Gaussian Family)
3+
4+
## Current State
5+
6+
The `Feglm` class (in `feglm_.py`) currently raises `NotImplementedError` at line 99-100 when fixed effects are present. However, the infrastructure is largely in place:
7+
8+
- The `residualize()` method exists (lines 304-323) and correctly calls `demean()`
9+
- Fixed effects are already handled in `prepare_model_matrix()` and `to_array()` methods
10+
- Separation checks (lines 102-126) are already conditional on `self._fe is not None`
11+
12+
The `Fepois` class successfully implements fixed effects in its IRLS algorithm (lines 282-294 in `fepois_.py`) by calling `demean()` with fixed effects and weights at each iteration.
13+
14+
## Implementation Steps
15+
16+
### 1. Remove Fixed Effects Restriction in feglm_.py
17+
18+
Remove the restriction in `prepare_model_matrix()` method:
19+
20+
- Delete lines 99-100 that raise `NotImplementedError` for fixed effects
21+
- This will allow the rest of the code to execute when fixed effects are present
22+
23+
### 2. Verify residualize() Method Integration
24+
25+
The `residualize()` method (lines 304-323) already properly handles fixed effects:
26+
27+
- Returns unchanged data when `flist is None` (no fixed effects)
28+
- Calls `demean()` with weights when fixed effects exist
29+
- This method is already called in `get_fit()` at line 182
30+
31+
The implementation should work as-is since:
32+
33+
- `Fegaussian` inherits from `Feglm`
34+
- The IRLS algorithm in `get_fit()` calls `residualize()` at each iteration (line 182)
35+
- Weights are properly passed as `W_tilde.flatten()` (line 186)
36+
37+
### 3. Test the Implementation
38+
39+
Enable and extend existing tests:
40+
41+
**a) Enable skipped test in `test_feols_feglm_internally.py`:**
42+
43+
- Remove the `@pytest.mark.skip` decorator on line 60
44+
- Run the test to verify it passes with the implementation
45+
46+
**b) Add basic R comparison tests:**
47+
48+
- Add tests to `test_vs_fixest.py` comparing `pf.feglm(family="gaussian")` against R's `fixest::feglm(family="gaussian")`
49+
- Test formulas: `"Y ~ X1 | f1"`, `"Y ~ X1 | f1 + f2"`, `"Y ~ X1 + X2 | f2"`
50+
- Compare: coefficients, standard errors, residuals
51+
- Use existing test patterns from the `glm_fmls` tests (around line 119)
52+
53+
### 4. Validation Checklist
54+
55+
Ensure the implementation correctly handles:
56+
57+
- Single fixed effect: `Y ~ X1 | f1`
58+
- Multiple fixed effects: `Y ~ X1 | f1 + f2`
59+
- Fixed effects with multiple covariates: `Y ~ X1 + X2 | f1`
60+
- Proper weight handling in demeaning at each IRLS iteration
61+
- Convergence behavior (should still converge in 1 iteration for Gaussian)
62+
63+
## Key Files to Modify
64+
65+
- `pyfixest/estimation/feglm_.py` (remove restriction)
66+
- `tests/test_feols_feglm_internally.py` (enable skipped test)
67+
- `tests/test_vs_fixest.py` (add R comparison tests)
68+
69+
## Expected Behavior
70+
71+
After implementation, `pf.feglm(fml="Y ~ X1 | f1", family="gaussian")` should:
72+
73+
1. Accept the formula without raising `NotImplementedError`
74+
2. Demean variables by fixed effects at each IRLS iteration
75+
3. Produce identical results to R's `fixest::feglm()` with Gaussian family
76+
4. For Gaussian family, should effectively match `feols()` results (with appropriate inference adjustments)
77+
78+
### To-dos
79+
80+
- [ ] Remove NotImplementedError for fixed effects in feglm_.py (lines 99-100)
81+
- [ ] Remove @pytest.mark.skip decorator from test_feols_feglm_internally
82+
- [ ] Add R comparison tests for Gaussian GLM with fixed effects to test_vs_fixest.py
83+
- [ ] Run tests and validate against R fixest results

.cursor/rules/pixi.mdc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
description: Running unit tests
3+
alwaysApply: false
4+
---
5+
When running unit tests, always:
6+
7+
* use pixi to run the test. We have a dev environement called "dev" that should house all dependencies.
8+
9+
When running unit tests, never:
10+
11+
* Alter the tolerance to get the test to pass. Use a default tolerance of 1e-06 when comparing two floats.

0 commit comments

Comments
 (0)