Skip to content

Conversation

@jakepenzak
Copy link
Contributor

Motivation:

  • To enable usage of RScorer class for scoring and model ensembling in the presence of discrete outcomes

Implementation:

  • Add discrete_outcome attribute to RScorer class and pass to LinearDML in RScorer.fit() method

File modifications:

  • econml/score/rscorer.py - Added discrete_outcome attribute
  • econml/tests/test_rscorer.py - Added test case for discrete outcomes and CATEs that take probabilistic form

@fverac
Copy link
Collaborator

fverac commented Nov 26, 2024

Code looks ready to merge.

The DCO check is failing though, which probably means you'll have to rebase the branch. Can you follow the instructions here?
https://github.com/py-why/EconML/pull/927/checks?check_run_id=33428718307

@jakepenzak
Copy link
Contributor Author

Code looks ready to merge.

The DCO check is failing though, which probably means you'll have to rebase the branch. Can you follow the instructions here? https://github.com/py-why/EconML/pull/927/checks?check_run_id=33428718307

Done.

Thanks for the reviewing this PR 🤜 🤛

@fverac
Copy link
Collaborator

fverac commented Nov 26, 2024

Hm seems like we're not done yet.

Says the branch is out-of-date with main. I think the problem is that in the current rebase there is a commit that merged in some updates from main ("pre commit autoupdate"), and git is not liking that.

Instead, can you update your local main branch and rebase from there?

jakepenzak and others added 4 commits November 26, 2024 20:57
Signed-off-by: Jacob Pieniazek <[email protected]>
Co-authored-by: fverac <[email protected]>
Signed-off-by: Jacob Pieniazek  <[email protected]>
Signed-off-by: Jacob Pieniazek <[email protected]>
@jakepenzak
Copy link
Contributor Author

Hm seems like we're not done yet.

Says the branch is out-of-date with main. I think the problem is that in the current rebase there is a commit that merged in some updates from main ("pre commit autoupdate"), and git is not liking that.

Instead, can you update your local main branch and rebase from there?

Ah yes, I missed that on my end. I believe everything should be good to go from here.

@fverac
Copy link
Collaborator

fverac commented Nov 26, 2024

Merging now, thanks for the contribution!

@fverac fverac merged commit a6fc2e6 into py-why:main Nov 26, 2024
92 checks passed
carl-offerfit pushed a commit to carl-offerfit/EconML that referenced this pull request Mar 4, 2025
* api(enable discrete outcome in RScorer)

Signed-off-by: Jacob Pieniazek <[email protected]>

* test(update DGP for test)

Signed-off-by: Jacob Pieniazek <[email protected]>

* Update econml/tests/test_rscorer.py

Co-authored-by: fverac <[email protected]>
Signed-off-by: Jacob Pieniazek  <[email protected]>
Signed-off-by: Jacob Pieniazek <[email protected]>

* test(rlearner): Update test data dimensions in _get_data to allow testing w/ SLearner

Signed-off-by: Jacob Pieniazek <[email protected]>

---------

Signed-off-by: Jacob Pieniazek <[email protected]>
Signed-off-by: Jacob Pieniazek  <[email protected]>
Co-authored-by: fverac <[email protected]>
Signed-off-by: Carl Gold <[email protected]>
carl-offerfit pushed a commit to carl-offerfit/EconML that referenced this pull request Mar 6, 2025
* api(enable discrete outcome in RScorer)

Signed-off-by: Jacob Pieniazek <[email protected]>

* test(update DGP for test)

Signed-off-by: Jacob Pieniazek <[email protected]>

* Update econml/tests/test_rscorer.py

Co-authored-by: fverac <[email protected]>
Signed-off-by: Jacob Pieniazek  <[email protected]>
Signed-off-by: Jacob Pieniazek <[email protected]>

* test(rlearner): Update test data dimensions in _get_data to allow testing w/ SLearner

Signed-off-by: Jacob Pieniazek <[email protected]>

---------

Signed-off-by: Jacob Pieniazek <[email protected]>
Signed-off-by: Jacob Pieniazek  <[email protected]>
Co-authored-by: fverac <[email protected]>
Signed-off-by: Carl Gold <[email protected]>
carl-offerfit pushed a commit to carl-offerfit/EconML that referenced this pull request Mar 31, 2025
* api(enable discrete outcome in RScorer)

Signed-off-by: Jacob Pieniazek <[email protected]>

* test(update DGP for test)

Signed-off-by: Jacob Pieniazek <[email protected]>

* Update econml/tests/test_rscorer.py

Co-authored-by: fverac <[email protected]>
Signed-off-by: Jacob Pieniazek  <[email protected]>
Signed-off-by: Jacob Pieniazek <[email protected]>

* test(rlearner): Update test data dimensions in _get_data to allow testing w/ SLearner

Signed-off-by: Jacob Pieniazek <[email protected]>

---------

Signed-off-by: Jacob Pieniazek <[email protected]>
Signed-off-by: Jacob Pieniazek  <[email protected]>
Co-authored-by: fverac <[email protected]>
Signed-off-by: Carl Gold <[email protected]>
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.

2 participants