Skip to content

[Repo Assist] fix: raise ValueError for unsupported identifier_method in covariate adjustment estimators#1442

Merged
emrekiciman merged 3 commits intomainfrom
repo-assist/fix-issue-618-estimator-identification-validation-068f80d9d620c11b
Apr 13, 2026
Merged

[Repo Assist] fix: raise ValueError for unsupported identifier_method in covariate adjustment estimators#1442
emrekiciman merged 3 commits intomainfrom
repo-assist/fix-issue-618-estimator-identification-validation-068f80d9d620c11b

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot commented Apr 6, 2026

🤖 This PR was created by Repo Assist, an automated AI assistant.

Closes #618

Root Cause

Covariate-adjustment estimators (LinearRegressionEstimator, GeneralizedLinearModelEstimator, all PropensityScore* estimators, and DistanceMatchingEstimator) silently produced incorrect results when used with incompatible identification strategies such as frontdoor, iv, or mediation.

When identifier_method is "frontdoor", get_adjustment_set() falls back to returning empty backdoor variables, causing the estimator to run without the right covariates and yielding a biased, incorrect effect estimate — without any error or warning.

DoublyRobustEstimator already has this guard; the other covariate-adjustment estimators did not.

Fix

Add an early ValueError to the fit() methods of:

  • RegressionEstimator (covers LinearRegressionEstimator and GeneralizedLinearModelEstimator)
  • PropensityScoreEstimator (covers PropensityScoreMatchingEstimator, PropensityScoreWeightingEstimator, PropensityScoreStratificationEstimator)
  • DistanceMatchingEstimator

The check fires before any computation and gives a clear message directing users to TwoStageRegressionEstimator (for frontdoor/mediation) or InstrumentalVariableEstimator (for iv).

TwoStageRegressionEstimator is unaffected because it already overrides identifier_method to "backdoor" on the internal estimand copies it passes to regression sub-estimators.

Tests

Added test_invalid_identifier_method_raises to TestLinearRegressionEstimator, parametrized over frontdoor, iv, and mediation.

Test Status

tests/causal_estimators/ — 31 passed in 591s

All estimator tests pass, including the existing two-stage regression (frontdoor) tests which confirm the internal identifier_method = "backdoor" override is unaffected by this change.

Note

🔒 Integrity filter blocked 55 items

The following items were blocked because they don't meet the GitHub integrity level.

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@b897c2f3e43bde9ff7923c8fa9211055b26e27cc

…adjustment estimators

Estimators based on covariate adjustment (linear regression, generalized
linear model, propensity score methods, distance matching) silently produced
incorrect results when used with incompatible identification strategies
(frontdoor, iv, mediation).

Add a clear ValueError to RegressionEstimator.fit(),
PropensityScoreEstimator.fit(), and DistanceMatchingEstimator.fit() that
fires before any computation when identifier_method is not one of
'backdoor' or 'general_adjustment'. This follows the same pattern already
established by DoublyRobustEstimator.

Also add parametrized tests to TestLinearRegressionEstimator that verify
the ValueError is raised for 'frontdoor', 'iv', and 'mediation'.

Closes #618

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added automation bug Something isn't working enhancement New feature or request repo-assist labels Apr 6, 2026
@emrekiciman emrekiciman marked this pull request as ready for review April 7, 2026 09:21
When using the functional API (identify_effect()), identifier_method is
not set and defaults to None. The previous whitelist check would raise a
ValueError for None, breaking the functional API notebook test.

Change the check to only raise for explicitly unsupported methods
(frontdoor, iv, mediation) while allowing None to pass through.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor Author

github-actions bot commented Apr 9, 2026

Commit pushed: 62d434c

Generated by Repo Assist

@github-actions
Copy link
Copy Markdown
Contributor Author

github-actions bot commented Apr 9, 2026

🤖 This is an automated update from Repo Assist.

CI fix: Pushed commit 3fa5c39 to address the notebook test failure (dowhy_functional_api.ipynb).

Root cause of the failure: The validation check used a whitelist (identifier_method not in ["backdoor", "general_adjustment"]). When using the functional API (identify_effect() rather than CausalModel.identify_effect()), identifier_method is never set and remains None. None is not in the whitelist, so the new guard was incorrectly raising ValueError.

Fix: Changed the check to identifier_method is not None and identifier_method not in [...]. This:

  • Allows None through (functional API case — effectively backdoor-like)
  • Still raises ValueError for explicit "frontdoor", "iv", "mediation" (the original intent)

Also added a regression test test_none_identifier_method_does_not_raise to TestLinearRegressionEstimator to prevent this from regressing.

Note

🔒 Integrity filter blocked 75 items

The following items were blocked because they don't meet the GitHub integrity level.

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@b897c2f3e43bde9ff7923c8fa9211055b26e27cc

Copy link
Copy Markdown
Member

@amit-sharma amit-sharma left a comment

Choose a reason for hiding this comment

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

good check to add

@emrekiciman emrekiciman merged commit f3fe766 into main Apr 13, 2026
37 checks passed
@emrekiciman emrekiciman deleted the repo-assist/fix-issue-618-estimator-identification-validation-068f80d9d620c11b branch April 13, 2026 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation bug Something isn't working enhancement New feature or request repo-assist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

check valid estimators for different identification strategies

2 participants