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

Cython implementation of GRF and CausalForestDML #341

Merged
merged 129 commits into from
Jan 9, 2021
Merged

Cython implementation of GRF and CausalForestDML #341

merged 129 commits into from
Jan 9, 2021

Conversation

vsyrgkanis
Copy link
Collaborator

This PR implements a high-performance Cython version of Generalized Random Forests and enables ForestDML for multiple treatments and multiple outcomes. Replaces and deprecates SubsampledHonestForest. Also deprecates ForestDML by CausalForestDML and CausalForest by CausalForestDML. Adds CausalIVForest for future integration with DMLIV.

@vasilismsr
added backend option in causal forest
5b88c6a
@vasilismsr
added backend option in causal forest
ff88cde
@vasilismsr
adding verbosity, restructuring static functions
b20f20f
@vasilismsr
started cython causal forests
57e4fe0
@vasilismsr
added version of grf criterion that optimizes mse and reweights heter…
a387f9c 
…ogeneity by jacobian matrix.
@vasilismsr
notebook update
82ae834
@vasilismsr
added a heterogeneity importance score that seems more robust for grf…
42eaa1c 
…. Added notebook with IVForest.
@vasilismsr
added notebooks that implement the IV grf and the local linear grf.
341c34e
@vasilismsr
added local linear IV
28c77b9
@vasilismsr
added python wrapper classes for grf
ce2c1e8
@vasilismsr
clean notebook
66ebf66
@vasilismsr
clean notebook
526f59f
@vasilismsr
switched forest prediction to the correct mean over mean
2ea9441
@vasilismsr
made het criterion also proxy_children_impurity=True
8e2aa88
@vasilismsr
made het criterion also proxy_children_impurity=True
a8496f9
@vasilismsr
fixed randomness in train_test split. Was using same exact seed as ra…
fbd5166 
…ndomness for subsampling causing weird correlations and non pseudorandomness
@vasilismsr
notebook
e682493
@vasilismsr
simplified cython code to deal with val and train
c331701
@vasilismsr
implemented first version of predict interval
ef4fd82
@vasilismsr
added halfsample inference
aae5229
@vasilismsr
cleaned up notebook
da46c7e
@vasilismsr
speed improvements
9a5f246
@vasilismsr
notebook
e0aff49
@vasilismsr
more speed ups
ca22b95
@vasilismsr
speed ups
f340e34
@vasilismsr
further speed ups. squeezing it out :)
5e1112d
@vasilismsr
switched the way tree fitting is spawn for speed improvement
e11edce
@vasilismsr
fixed variable importance. Changed het criterion to only use the rele…
acb5a6e 
…vant output heterogeneity
@vasilismsr
fixed balance of halfsamples and subsamples.
7c08f72
@vasilismsr
added option to not fit intercept
5a59ad6
heimengqi
heimengqi suggested changes Jan 5, 2021
View reviewed changes
Copy link
Contributor

@heimengqi heimengqi left a comment

Choose a reason for hiding this comment

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

I reviewed each file related to shap and each file under dml folder. Mostly looks good, only minor issue needs to be addressed.
The logic for all the functions under shap.py are heavily related and repeated, I might need to do a second round improvement.

econml/__init__.py Show resolved Hide resolved
econml/shap.py Show resolved Hide resolved
econml/shap.py Outdated Show resolved Hide resolved
econml/shap.py Outdated Show resolved Hide resolved
econml/shap.py Show resolved Hide resolved
econml/tests/test_shap.py Outdated Show resolved Hide resolved
econml/tests/test_shap.py Outdated Show resolved Hide resolved
notebooks/Interpretability with SHAP.ipynb Outdated Show resolved Hide resolved
kbattocchi
kbattocchi requested changes Jan 6, 2021
View reviewed changes
Copy link
Collaborator

@kbattocchi kbattocchi left a comment

Choose a reason for hiding this comment

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

Mostly looks fine; added a few suggestions.

doc/reference.rst Show resolved Hide resolved
doc/spec/api.rst Show resolved Hide resolved
doc/spec/estimation/forest.rst Outdated Show resolved Hide resolved
doc/spec/estimation/forest.rst Show resolved Hide resolved
econml/__init__.py Outdated Show resolved Hide resolved
econml/drlearner.py Show resolved Hide resolved
econml/setup.py Outdated Show resolved Hide resolved
econml/tree/_splitter.pyx Outdated Show resolved Hide resolved
econml/tree/_utils.pyx Outdated Show resolved Hide resolved
econml/utilities.py Outdated Show resolved Hide resolved
@vsyrgkanis vsyrgkanis force-pushed the vasilis/grf_simplification branch 2 times, most recently from 980ec82 to d16b615 Compare January 6, 2021 20:23
@vasilismsr
partially addressing keith's review
84f76d1 
fixed shap improt

addressed Keith's review comments

fixed doctest

added TODO for sklearn compaitbility
@kbattocchi kbattocchi self-requested a review January 6, 2021 21:51
kbattocchi
kbattocchi approved these changes Jan 6, 2021
View reviewed changes
Copy link
Collaborator

@kbattocchi kbattocchi left a comment

Choose a reason for hiding this comment

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

LGTM

heimengqi
heimengqi reviewed Jan 7, 2021
View reviewed changes
econml/shap.py Outdated Show resolved Hide resolved
heimengqi
heimengqi approved these changes Jan 7, 2021
View reviewed changes
Copy link
Contributor

@heimengqi heimengqi 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!

moprescu
moprescu approved these changes Jan 8, 2021
View reviewed changes
Copy link
Contributor

@moprescu moprescu left a comment

Choose a reason for hiding this comment

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

Looks good, nothing stood out to me. Mainly looked at the GRF portion.

econml/causal_tree.py Outdated Show resolved Hide resolved
econml/ortho_forest.py Show resolved Hide resolved
econml/ortho_forest.py Outdated Show resolved Hide resolved
@vsyrgkanis vsyrgkanis merged commit bb042d5 into master Jan 9, 2021
@vsyrgkanis vsyrgkanis deleted the vasilis/grf_simplification branch January 9, 2021 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kbattocchi kbattocchi kbattocchi approved these changes

@moprescu moprescu moprescu approved these changes

@heimengqi heimengqi heimengqi approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vsyrgkanis @kbattocchi @moprescu @heimengqi @vasilismsr