py-why / dowhy

DoWhy is a Python library for causal inference that supports explicit modeling and testing of causal assumptions. DoWhy is based on a unified language for causal inference, combining causal graphical models and potential outcomes frameworks.
https://www.pywhy.org/dowhy
MIT License
6.99k stars 923 forks source link

support additional parameters in AddUnobservedCommonCause.refute_estimate #385

Open leo-ware opened 2 years ago

leo-ware commented 2 years ago

The current docstring for AddUnobservedCommonCause says the refute_estimate method accepts a number of parameters.

    - 'confounders_effect_on_treatment': how the simulated confounder affects the value of treatment. This can be linear (for continuous treatment) or binary_flip (for binary treatment)
    - 'confounders_effect_on_outcome': how the simulated confounder affects the value of outcome. This can be linear (for continuous outcome) or binary_flip (for binary outcome)
    - 'effect_strength_on_treatment': parameter for the strength of the effect of simulated confounder on treatment. For linear effect, it is the regression coeffient. For binary_flip, it is the probability that simulated confounder's effect flips the value of treatment from 0 to 1 (or vice-versa).
    - 'effect_strength_on_outcome': parameter for the strength of the effect of simulated confounder on outcome. For linear effect, it is the regression coeffient. For binary_flip, it is the probability that simulated confounder's effect flips the value of outcome from 0 to 1 (or vice-versa).

The method itself doesn't seem to support any of these arguments though, and it doesn't accept *args or **kwargs. I've added a message to the docstring clarifying that this is a WIP, but we should probably make a decision about whether we want these features or not, and then either implement them or remove them from the docstring.

amit-sharma commented 2 years ago

It does use these arguments. They are passed to __init__ method of the class. The docstring needs to be improved---the refute_estimate in the docstring refers to the user-facing refute_estimate from CausalModel. I think the docstring can be clarified. I've added a PR for better docstrings for estimators, where these parameters are now explicitly named in the __init__ method. Will do the same for refuters too.