Skip to content

Widen the propensity model's cross-validated C grid#944

Open
arpitjain099 wants to merge 1 commit into
uber:masterfrom
arpitjain099:chore/propensity-cs-grid
Open

Widen the propensity model's cross-validated C grid#944
arpitjain099 wants to merge 1 commit into
uber:masterfrom
arpitjain099:chore/propensity-cs-grid

Conversation

@arpitjain099

Copy link
Copy Markdown
Contributor

Proposed changes

No filed issue, I was reading through causalml/propensity and the Cs value on LogisticRegressionPropensityModel looked off:

"Cs": np.logspace(1e-3, 1 - 1e-3, 4),

That expands to about [1.00, 2.16, 4.64, 9.98], so every candidate is C >= 1. LogisticRegressionCV searches C (the inverse of regularization strength), so this grid only ever lets it choose weak regularization and it can never reach C < 1. The whole search sits inside a single order of magnitude on the weak side.

It reads like the 1e-3 / 1 - 1e-3 bounds got carried over from the l1_ratios line right below it (where a [0, 1] range is exactly right) and from clip_bounds. For Cs those numbers land in the exponent of logspace, which is what collapses the range.

I changed it to Cs=4, which hands the job back to LogisticRegressionCV: it builds its own log grid between 1e-4 and 1e4 and 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

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Added test_logistic_regression_propensity_model_cs_grid in tests/test_propensity.py. It fits the model and asserts the resulting model.Cs_ spans min < 1 < max. On master the grid is [1.00, 2.16, 4.64, 9.98] so it fails; with this change Cs_ is [1e-4, 4.6e-2, 21.5, 1e4] and it passes. black is 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.

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>
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.

1 participant