Widen the propensity model's cross-validated C grid#944
Open
arpitjain099 wants to merge 1 commit into
Open
Conversation
LogisticRegressionPropensityModel sets Cs=np.logspace(1e-3, 1 - 1e-3, 4), which evaluates to roughly [1.00, 2.16, 4.64, 9.98]. Every candidate is C >= 1, so LogisticRegressionCV can only ever pick weak regularization and never explores strong regularization (C < 1). The 1e-3 / 1 - 1e-3 bounds look copied from the neighboring l1_ratios line (where a [0, 1] range is correct) and from clip_bounds, but for Cs they land in the exponent, which collapses the search to a single order of magnitude. Pass Cs=4 instead and let LogisticRegressionCV build its own log-spaced grid between 1e-4 and 1e4, keeping four candidate values. The CV can now tune the penalty across strong and weak regularization. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
No filed issue, I was reading through
causalml/propensityand theCsvalue onLogisticRegressionPropensityModellooked off:That expands to about
[1.00, 2.16, 4.64, 9.98], so every candidate isC >= 1.LogisticRegressionCVsearchesC(the inverse of regularization strength), so this grid only ever lets it choose weak regularization and it can never reachC < 1. The whole search sits inside a single order of magnitude on the weak side.It reads like the
1e-3/1 - 1e-3bounds got carried over from thel1_ratiosline right below it (where a[0, 1]range is exactly right) and fromclip_bounds. ForCsthose numbers land in the exponent oflogspace, which is what collapses the range.I changed it to
Cs=4, which hands the job back toLogisticRegressionCV: it builds its own log grid between1e-4and1e4and keeps four points. Fitted models now search across both strong and weak regularization. This does shift the chosen penalty on existing pipelines, but toward the range the CV was meant to cover.Types of changes
Checklist
Further comments
Added
test_logistic_regression_propensity_model_cs_gridintests/test_propensity.py. It fits the model and asserts the resultingmodel.Cs_spansmin < 1 < max. On master the grid is[1.00, 2.16, 4.64, 9.98]so it fails; with this changeCs_is[1e-4, 4.6e-2, 21.5, 1e4]and it passes.blackis clean.Happy to use an explicit grid like
np.logspace(-4, 4, 4)instead if you'd rather keep it spelled out, or bump the point count.