-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add methods for performing Wald-tests #668
Conversation
`.covariance_matrix()` expects X and weights in a different format than what we have at the end of `.fit().
Can be re-added when we actually store term names in the model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I have some small stylistic points and a suggestion for a check that we could do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good.
Please look at my suggestion for renaming the wald test methods. Otherwise, I think this can be merged.
🎉
Thank you @MarcAntoineSchmidtQC! The methods have been renamed. Should it go into |
Note: windows conda-build also fails on main. (was fine in the morning...) |
Related mamba issue: mamba-org/mamba#2770 |
Are you pinning Mamba to 1.4.2 on purpose? |
I don't think so. I'm not even sure where that pin comes from. |
@jonashaag I don't think it's about the libmamba version. The CI for the same commit was green this morning but red when I rerun it this afternoon.. Based on the logs, the difference between the |
|
Never mind, libsolv build 2 was indeed broken and build 3 seem to have solved our issues |
Checklist
CHANGELOG.rst
entrySummary
This PR adds some support for doing inference on estimated GLMs. Wald-tests can be performed based on a restriction matrix and corresponding RHS vector (lower-level, flexible), a list of feature names
or a list of term names.Notes
This branch is based on thestore-cov-matrix
branch. Therefore, PR Add the option to store the covariance matrix to avoid recomputing it #661 should be merged before this one. I am opening this draft PR to get some feedback until that happens.Term names is not a concept in glum yet. The idea is based on Tabmat PR #278 and the closed Glum PR Create feature names for non-pandas input, too #655. Either these will have to be merged first, or theGeneralizedLinearRegressorBase.wald_test_term_name
method will have to be removed from this PR until term names are implemented.