Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable stratification in bootstrap #236

Merged
merged 5 commits into from
Nov 2, 2020
Merged

Enable stratification in bootstrap #236

merged 5 commits into from
Nov 2, 2020

Conversation

kbattocchi
Copy link
Collaborator

@kbattocchi kbattocchi commented Mar 25, 2020 •
edited
Loading

Fixes #125. Fixes #89.

@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch 2 times, most recently from 7b8054c to aeb6948 Compare March 26, 2020 21:48
@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch 3 times, most recently from 94a4cea to 715adbe Compare April 10, 2020 07:17
@kbattocchi kbattocchi marked this pull request as ready for review April 10, 2020 13:04
@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch from 6c7caba to 600a403 Compare May 22, 2020 16:44
@kbattocchi kbattocchi mentioned this pull request May 22, 2020
Closed
@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch from 600a403 to c561cd6 Compare May 29, 2020 15:22
@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch 2 times, most recently from b788c13 to 50c5613 Compare June 10, 2020 15:06
@kbattocchi kbattocchi mentioned this pull request Jun 10, 2020
Open
@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch 6 times, most recently from c0529c4 to 52eff24 Compare August 14, 2020 15:02
@vsyrgkanis
Copy link
Collaborator

The point estimate in the bootstrap should be the estimate on the original set of samples, not the mean of the bootstrap subsamples.

vsyrgkanis
vsyrgkanis requested changes Sep 8, 2020
View reviewed changes
econml/_ortho_learner.py Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/inference.py Outdated Show resolved Hide resolved
vsyrgkanis
vsyrgkanis requested changes Sep 8, 2020
View reviewed changes
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
vsyrgkanis
vsyrgkanis requested changes Sep 8, 2020
View reviewed changes
econml/bootstrap.py Outdated Show resolved Hide resolved
vsyrgkanis
vsyrgkanis requested changes Sep 8, 2020
View reviewed changes
econml/bootstrap.py Outdated Show resolved Hide resolved
vsyrgkanis
vsyrgkanis requested changes Sep 8, 2020
View reviewed changes
econml/inference.py Outdated Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
vsyrgkanis
vsyrgkanis requested changes Sep 8, 2020
View reviewed changes
econml/inference.py Outdated Show resolved Hide resolved
moprescu
moprescu approved these changes Sep 11, 2020
View reviewed changes
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/bootstrap.py Outdated Show resolved Hide resolved
econml/_ortho_learner.py Show resolved Hide resolved
@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch from 0490101 to 63daf39 Compare October 7, 2020 16:04
@kbattocchi kbattocchi requested review from vsyrgkanis and removed request for vasilismsr October 7, 2020 19:05
vsyrgkanis
vsyrgkanis approved these changes Oct 23, 2020
View reviewed changes
Copy link
Collaborator

@vsyrgkanis vsyrgkanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small change in marginal_effect_inference logic at the LinearCateEstimator level is required.

econml/cate_estimator.py Outdated Show resolved Hide resolved
@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch 2 times, most recently from c0e95f7 to 8084c0a Compare October 29, 2020 19:09
vsyrgkanis
vsyrgkanis reviewed Oct 30, 2020
View reviewed changes
Copy link
Collaborator

@vsyrgkanis vsyrgkanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these really the only places where const_marginal_effect_interval and inference are defined, or are there many other places where the signature needs to change??

@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch from 8084c0a to 11c9dac Compare October 30, 2020 19:44
vsyrgkanis
vsyrgkanis approved these changes Oct 30, 2020
View reviewed changes
Copy link
Collaborator

@vsyrgkanis vsyrgkanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@kbattocchi kbattocchi force-pushed the kebatt/bootstrapFixes branch from 11c9dac to f4f0bf5 Compare October 30, 2020 23:46
@kbattocchi kbattocchi merged commit 6fbc585 into master Nov 2, 2020
@kbattocchi kbattocchi deleted the kebatt/bootstrapFixes branch November 2, 2020 02:30
@kbattocchi kbattocchi mentioned this pull request Nov 9, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moprescu moprescu moprescu approved these changes

@vsyrgkanis vsyrgkanis vsyrgkanis approved these changes

@heimengqi heimengqi Awaiting requested review from heimengqi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bootstrap must be stratified for discrete treatments Enable a greater variety of bootstrap estimates
3 participants
@kbattocchi @vsyrgkanis @moprescu