Bugfix rscorer discrete outcome treatment#977
Merged
kbattocchi merged 1 commit intopy-why:mainfrom Jun 7, 2025
Merged
Conversation
Signed-off-by: maartenvanhooft <mjgvanhooft@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where the discrete_outcome flag was not forwarded to RScorer in CausalForestDML.tune and adds a minimal test to ensure tuning and fitting succeed when both outcome and treatment are discrete.
- Pass
discrete_outcomeintoRScorerduring tuning. - Add a test that runs
tuneandfitwithdiscrete_outcome=Trueanddiscrete_treatment=True.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| econml/dml/causal_forest.py | Included discrete_outcome=est.discrete_outcome in RScorer instantiation within tune. |
| econml/tests/test_dml.py | Added test_causal_forest_tune_with_discrete_outcome_and_treatment to verify no errors when both flags are true. |
Comments suppressed due to low confidence (1)
econml/tests/test_dml.py:1304
- [nitpick] The new test does not include any assertions to verify the estimator's behavior. Consider adding checks—for example, assert that the internally inferred outcome dimension (
est._d_y) matches the expected number of classes or that callingest.effect(X)returns an array of the correct shape—to ensure discrete flags are honored.
def test_causal_forest_tune_with_discrete_outcome_and_treatment(self):
kbattocchi
approved these changes
Jun 6, 2025
Member
kbattocchi
left a comment
There was a problem hiding this comment.
Thanks for the contribution, this looks great. Once our checks pass I'll merge it.
carl-offerfit
pushed a commit
to carl-offerfit/EconML
that referenced
this pull request
Jul 7, 2025
Signed-off-by: maartenvanhooft <mjgvanhooft@gmail.com> Signed-off-by: Carl Gold <carl.goldd@braze.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.
Issue #875
Changes:
discrete_outcomeflag to RScorer. As a result, it gets initiated by it's default valueFalse, which is unintended behaviour if we wantdiscrete_outcome=TrueFirst contribution here, please review critically for any mistakes or inconsistencies!
I see that the CI pipeline awaits approval. I think I did everything that is expected from me, so I'll mark it as ready for review. But perhaps the pipeline flags something.