-
Notifications
You must be signed in to change notification settings - Fork 1k
[Repo Assist] fix: update second-stage model estimand when pre-instantiated in TwoStageRegression #1462
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
base: main
Are you sure you want to change the base?
[Repo Assist] fix: update second-stage model estimand when pre-instantiated in TwoStageRegression #1462
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -124,9 +124,7 @@ def test_frontdoor_estimator(self): | |
| target "X" | ||
| ] | ||
| ] | ||
| """.replace( | ||
| "\n", "" | ||
| ) | ||
| """.replace("\n", "") | ||
|
|
||
| N_SAMPLES = 10000 | ||
| # Generate the data | ||
|
|
@@ -209,9 +207,7 @@ def _make_mediation_data(n=2000, seed=42): | |
| edge [ source "X" target "Y" ] | ||
| edge [ source "M" target "Y" ] | ||
| ] | ||
| """.replace( | ||
| "\n", " " | ||
| ) | ||
| """.replace("\n", " ") | ||
|
|
||
|
|
||
| class TestTwoStageRegressionMediationNIE: | ||
|
|
@@ -316,3 +312,52 @@ def test_nde_estimand_uses_correct_backdoor_variables(self): | |
| nde_estimand = estimator._second_stage_model_nde._target_estimand | ||
| assert nde_estimand.identifier_method == "backdoor" | ||
| assert nde_estimand.backdoor_variables == estimand.mediation_second_stage_confounders | ||
|
|
||
|
|
||
| class TestTwoStageRegressionPreinstantiatedSecondStage: | ||
| """Regression tests for #1335: KeyError when second_stage_model is a pre-instantiated CausalEstimator. | ||
|
|
||
| When a user passes an already-constructed estimator instance as second_stage_model, | ||
| the TwoStageRegressionEstimator must update its _target_estimand to use the | ||
| modified (backdoor) estimand rather than the original mediation estimand. | ||
| """ | ||
|
|
||
| def test_nie_with_preinstantiated_second_stage_no_keyerror(self): | ||
| """Passing a pre-instantiated second_stage_model must not raise KeyError.""" | ||
| import statsmodels.api as sm | ||
|
|
||
| from dowhy.causal_estimators.generalized_linear_model_estimator import GeneralizedLinearModelEstimator | ||
|
|
||
| df = _make_mediation_data() | ||
| model = CausalModel(data=df, treatment="X", outcome="Y", graph=_MEDIATION_GML) | ||
| estimand = model.identify_effect( | ||
| estimand_type=EstimandType.NONPARAMETRIC_NIE, | ||
| proceed_when_unidentifiable=True, | ||
| ) | ||
| second_stage = GeneralizedLinearModelEstimator(identified_estimand=estimand, glm_family=sm.families.Gaussian()) | ||
| # This must not raise KeyError: None | ||
| estimate = model.estimate_effect( | ||
| identified_estimand=estimand, | ||
| method_name="mediation.two_stage_regression", | ||
| method_params={"second_stage_model": second_stage}, | ||
| ) | ||
| assert np.isfinite(estimate.value) | ||
|
|
||
| def test_nie_preinstantiated_second_stage_estimand_updated(self): | ||
| """The pre-instantiated second_stage_model's _target_estimand is updated to backdoor.""" | ||
| import statsmodels.api as sm | ||
|
|
||
| from dowhy.causal_estimators.generalized_linear_model_estimator import GeneralizedLinearModelEstimator | ||
|
|
||
| df = _make_mediation_data() | ||
| model = CausalModel(data=df, treatment="X", outcome="Y", graph=_MEDIATION_GML) | ||
| estimand = model.identify_effect( | ||
| estimand_type=EstimandType.NONPARAMETRIC_NIE, | ||
| proceed_when_unidentifiable=True, | ||
| ) | ||
| second_stage = GeneralizedLinearModelEstimator(identified_estimand=estimand, glm_family=sm.families.Gaussian()) | ||
| estimator = TwoStageRegressionEstimator( | ||
| identified_estimand=estimand, | ||
| second_stage_model=second_stage, | ||
| ) | ||
| assert estimator._second_stage_model._target_estimand.identifier_method == "backdoor" | ||
|
Comment on lines
+329
to
+367
|
||
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.
The pre-instantiated-model handling was fixed for
second_stage_model, butfirst_stage_modelstill accepts a pre-instantiatedCausalEstimatorwithout updating its_target_estimandto the backdoor-modified estimand. If a user passes a pre-instantiatedRegressionEstimator(e.g.,GeneralizedLinearModelEstimator) for the first stage with a mediation estimand,RegressionEstimator.fit()will raise (identifier_method='mediation') or hit the samedefault_backdoor_id=Noneissue. Consider applying the same pattern used below forsecond_stage_modelto thefirst_stage_modelbranch as well (i.e., setself._first_stage_model._target_estimandto the first-stagemodified_target_estimand).