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.

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

inference='auto' #205

Closed
vasilismsr opened this issue Dec 21, 2019 · 3 comments · Fixed by #307
Closed

inference='auto' #205

vasilismsr opened this issue Dec 21, 2019 · 3 comments · Fixed by #307
Assignees
Labels
enhancement New feature or request

Comments

@vasilismsr
Copy link
Contributor

We should add a "inference='auto'" or "inference=True" option to all estimators, which would use some default option for inference. For instance, this could be bootstrap or None (if we want to disallow inference in some estimators that we think bootstrap will be deceiving) or the more tailored methods.

This will allow the user to not require to know which one is the most appropriate method. Also it would allow one to not have to change the fit call whenever they change the estimator class.

@vasilismsr vasilismsr added the enhancement New feature or request label Dec 21, 2019
@arose13
Copy link
Contributor

arose13 commented Mar 26, 2020

Hey @vasilismsr, for even medium sized datasets or complex models like Random Forests or GBMs automatically enabling inference to do something, worse case bootstrap can be prohibitively expensive computational.

I guess what I'm saying is, if the user doesn't know whether a particular inference method paired with their chosen models would be expensive or it, it should not be automated for them because from the user point of view it will look like the process gets stuck on the .fit() call.

Obviously please correct me if I'm wrong.

@arose13
Copy link
Contributor

arose13 commented Mar 26, 2020

For example in my package which has this package as a dependency I replace the inference kwarg with bootstrap_inference argument which accepts a Boolean and pick the appropriate method internally

@vasilismsr
Copy link
Contributor Author

Thanks @arose13 ! Indeed, maybe having bootstrap as a default is a bad idea. Though we have some particular subclasses where inference is at no computational cost and there is an obvious default method. For instance, in LinearDMLCateEstimator setting inference='statsmodels' is the obvious option and has no computational overhead. Similarly, for SparseLinearDMLCateEstimator it is inference='debiasedlasso' and for ForestDMLCateEstimator it is inference='blb'. Similarly for the corresponding DRLearner and DRIV estimators.

Would then a solution of the form: inference='auto', by default means for most classes inference=None and for these classes where it is clear what the prefered method is, we set it to that? Would that break the dependency in your package?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants