-
Notifications
You must be signed in to change notification settings - Fork 741
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.
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
Cython implementation of GRF and CausalForestDML #341
Conversation
…ogeneity by jacobian matrix.
…. Added notebook with IVForest.
…ndomness for subsampling causing weird correlations and non pseudorandomness
…vant output heterogeneity
…ed shap to use input featurenames
There was a problem hiding this 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.
There was a problem hiding this 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.
980ec82
to
d16b615
Compare
fixed shap improt addressed Keith's review comments fixed doctest added TODO for sklearn compaitbility
d16b615
to
84f76d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
There was a problem hiding this 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.
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.