Skip to content

Add offset option for fepois#805

Open
bradhackinen wants to merge 5 commits intopy-econometrics:masterfrom
bradhackinen:master
Open

Add offset option for fepois#805
bradhackinen wants to merge 5 commits intopy-econometrics:masterfrom
bradhackinen:master

Conversation

@bradhackinen
Copy link
Copy Markdown

This code adds an offset option to fepois regressions so that the function can be used to model differences in outcome rates.

  • In terms of the UI, I followed the functionality of the weights parameter in feols regressions. So the offset argument takes a string that must refer to a column name in the data. If no argument is supplied, the _offset vector is set to 0. This required modifying model_matrix_fixest to take an optional offset argument.
  • Changes to the estimation code are very simple. The offset is added to Z before demeaning and WLS, and then re-added to eta before computing the deviance.

I tested the code manually on the included poisson test data, a separation example (01.csv), and data from a current research project. The output the matches the R implementation of fixest (at least to the decimal places visible in the output) in with and without FE.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 4, 2025

Codecov Report

❌ Patch coverage is 61.11111% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...xest/estimation/deprecated/model_matrix_fixest_.py 0.00% 6 Missing ⚠️
pyfixest/estimation/models/fepois_.py 88.88% 1 Missing ⚠️
Flag Coverage Δ
core-tests 70.98% <50.00%> (-0.05%) ⬇️
test-r-core 52.60% <61.11%> (+0.01%) ⬆️
test-r-extended 18.72% <0.00%> (-0.04%) ⬇️
test-r-fixest 39.89% <61.11%> (+0.04%) ⬆️
tests-extended ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pyfixest/estimation/FixestMulti_.py 82.29% <100.00%> (+0.28%) ⬆️
pyfixest/estimation/api/fepois.py 96.00% <ø> (ø)
pyfixest/estimation/models/fepois_.py 78.57% <88.88%> (+0.12%) ⬆️
...xest/estimation/deprecated/model_matrix_fixest_.py 11.70% <0.00%> (-0.24%) ⬇️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@s3alfisc
Copy link
Copy Markdown
Member

s3alfisc commented Feb 4, 2025

pre-commit.ci autofix

@s3alfisc
Copy link
Copy Markdown
Member

s3alfisc commented Feb 7, 2025

Hi @bradhackinen , looks like the unit tests I added fail, but almost pass, which is of course the worst that could happen as it isn't clear if the implementation is slightly off or the tolerance just too low 😅 . I will try to debug this in the next days, but might not find time this weekend as I am traveling.

@bradhackinen
Copy link
Copy Markdown
Author

No worries @s3alfisc. Thanks for writing the tests. Let me know if there is something I can do to help resolve the problems. I can get rpy2 set up on my system next week if necessary.

@marie-gar
Copy link
Copy Markdown

Hi @bradhackinen ! I believe the test failures on this PR may have been caused by a bug that has since been fixed in #900 (the dependent variable was being cast to integer in Poisson regression, causing information loss). We've been using your branch with that fix applied and the offset feature works as expected on our end. Could you please try rebasing on the latest master and rerunning the CI? Thanks in advance!

@s3alfisc
Copy link
Copy Markdown
Member

Awesome, thanks for the info @marie-gar! I will take a look at this in the next days (unless @bradhackinen feels like he wants to take another stab at it). Brad - my apologies I kept this open for so long! I vaguely recall that I was busy implementing other things, and on top of that I had never used offsets and needed to do some reading first, which I of course never got too ...

@s3alfisc
Copy link
Copy Markdown
Member

Actually, we moved quite a bit of code around, so likely best if I were to try to do the rebase?

@bradhackinen
Copy link
Copy Markdown
Author

Sure, @s3alfisc, I'm happy to let you take the lead on integrating this. But let me know if there is something I can do to help.

@s3alfisc
Copy link
Copy Markdown
Member

Tests seem to run now, how does it look to you @marie-gar @bradhackinen ?

@marie-gar
Copy link
Copy Markdown

Awesome! Thanks @s3alfisc, @bradhackinen !

@s3alfisc
Copy link
Copy Markdown
Member

s3alfisc commented Apr 1, 2026

I somehow lost track of this again - I will merge and release this over the Easter break =)

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