Skip to content

Commit 612a3f1

Browse files
feat(smart): g_computation in compare_estimators + Sprint-B method labels in sensitivity_dashboard
Round-3 Smart-tool integration pass for Sprint-B. 1. sp.compare_estimators — adds 'g_computation' as a first-class comparison method. Each branch now has an explicit binary-treat guard that emits a compact warning on continuous D rather than falling through to sp.g_computation's own 20 KB value-dump ValueError (which the outer try/except would otherwise swallow inside a multi-kilobyte UserWarning). The docstring explicitly lists the four Sprint-B methods that are NOT supported here (proximal, msm, principal_strat, mediation family) because each needs extra kwargs beyond the shared signature; callers can still call those estimators directly. 2. sp.sensitivity_dashboard — method-label lookup reads ``.method`` → ``model_info['model_type']`` → ``model_info['estimator']`` → 'Unknown'. Before: Sprint-B CausalResult objects rendered as 'Unknown' in the dashboard header because they store the label on ``.method`` rather than ``model_info['model_type']``. Now the dashboard prints e.g. 'Proximal Causal Inference (linear 2SLS)'. 3. docs/ROADMAP.md §6 — new entry documenting the Sprint-B gap in compare_estimators, with a fully-specified collision rule for the planned ``method_hints`` extension (per-method hints take precedence over shared kwargs; UserWarning on conflict). 4. tests/test_smart_tools_sprint_b_round3.py (8 tests) — contract tests for each of the above, including a leak-free unsupported- method test (no permissive try/except; relies on compare_estimators's own per-branch exception wrapping) and a binary-guard warning-size test (<500 chars, not 20 KB). Two findings from external code review applied: test no-longer- permissive, explicit binary guard in g_computation branch. One minor (ROADMAP collision rule) clarified with explicit per-method-wins + UserWarning-on-conflict spec. Regression: 188/188 Sprint-B tests pass; 78/78 broader smart-tool regression tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ae4eac1 commit 612a3f1

4 files changed

Lines changed: 285 additions & 2 deletions

File tree

docs/ROADMAP.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,3 +170,44 @@ release that bumps to 1.0 (alignment is a 1.0 story).
170170
* Flip the default in the release after that.
171171

172172
---
173+
174+
## 6 · `sp.compare_estimators` — hint-aware Sprint-B support
175+
176+
**Shipped in 0.9.x**: ``sp.compare_estimators`` accepts ``methods=``
177+
from ``{'ols', 'matching', 'ipw', 'aipw', 'dml', 'g_computation',
178+
'causal_forest', 'did', 'panel_fe'}``. Each branch shares the same
179+
``(data, y, treatment, covariates, id, time, instrument)`` signature.
180+
181+
**Deferred**: the four Sprint-B estimators that need extra arguments
182+
the shared signature does not expose — ``'proximal'`` (needs
183+
``proxy_z`` + ``proxy_w``), ``'msm'`` (needs ``time_varying``),
184+
``'principal_strat'`` (needs a post-treatment ``strata`` column), and
185+
the mediation family (needs ``mediator`` and optionally
186+
``tv_confounders``). Calling them requires bespoke args per method.
187+
188+
**Why deferred**: bloating the shared signature with a dozen
189+
optional kwargs would regress the ergonomics for the common
190+
methods, while a hint-forwarding map (``method_hints={'proximal':
191+
{'proxy_z': [...], 'proxy_w': [...]}}``) needs design work to avoid
192+
confusing multi-method semantics.
193+
194+
**Trigger**: a user request to compare proximal against DML / TMLE
195+
on the same data set.
196+
197+
**Rough design**:
198+
199+
* Add ``method_hints: Dict[str, Dict[str, Any]]`` parameter that
200+
maps a method name to its per-method kwargs.
201+
* Dispatch each Sprint-B branch to the corresponding estimator using
202+
both the shared args and the per-method hints.
203+
* Keep backward compatibility: callers who don't pass
204+
``method_hints`` get the existing behaviour.
205+
* **Collision rule**: per-method hints take precedence over the
206+
shared kwargs for the method they name. If
207+
``covariates=['age']`` is the top-level shared arg and
208+
``method_hints={'proximal': {'covariates': ['age', 'educ']}}`` is
209+
supplied, proximal uses ``['age', 'educ']`` and every other method
210+
uses ``['age']``. Emit a ``UserWarning`` on conflict so the
211+
override is visible in the log.
212+
213+
---

src/statspai/smart/compare.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,15 @@ def compare_estimators(
132132
methods : list of str, optional
133133
Estimators to compare. Default auto-selects based on data.
134134
Options: 'ols', 'matching', 'ipw', 'aipw', 'dml',
135-
'causal_forest', 'did', 'panel_fe'.
135+
'g_computation', 'causal_forest', 'did', 'panel_fe'.
136+
137+
Not supported here (require extra arguments beyond the shared
138+
signature): ``'proximal'`` (needs proxy_z / proxy_w),
139+
``'msm'`` (needs time_varying / id / time),
140+
``'principal_strat'`` (needs a post-treatment strata variable),
141+
and the mediation family (needs a mediator). Call those
142+
estimators directly; see docs/ROADMAP.md for the planned
143+
hint-aware multi-method comparison surface.
136144
covariates : list of str, optional
137145
id : str, optional
138146
Panel unit ID.
@@ -211,6 +219,35 @@ def compare_estimators(
211219
results['AIPW'] = r
212220
name = 'Augmented IPW (DR)'
213221

222+
elif method == 'g_computation':
223+
# sp.g_computation uses the 'treat=' kwarg (not
224+
# 'treatment=') to match the rest of the Sprint-B
225+
# causal-inference surface. Handle that mapping here
226+
# so the shared ``treatment`` parameter works.
227+
# Explicit binary guard: the default ATE path in
228+
# g_computation rejects non-binary D. Without this
229+
# guard the outer ``except Exception`` below would
230+
# swallow the error in a 400-line ValueError dump of
231+
# unique treatment values. Raising a clean
232+
# ValueError here keeps the warning compact.
233+
treat_vals = df[treatment].dropna().unique()
234+
if not set(treat_vals).issubset({0, 1, 0.0, 1.0}):
235+
raise ValueError(
236+
f"g_computation requires a binary treatment "
237+
f"(0/1); '{treatment}' has "
238+
f"{len(treat_vals)} unique values "
239+
f"(first: {sorted(treat_vals)[:3]}). "
240+
f"Use estimand='dose_response' directly via "
241+
f"sp.g_computation() for continuous D."
242+
)
243+
r = sp.g_computation(df, y=y, treat=treatment,
244+
covariates=covariates[:10],
245+
seed=0)
246+
est = r.estimate
247+
se = r.se
248+
results['G-computation'] = r
249+
name = 'G-computation (parametric g-formula)'
250+
214251
elif method == 'dml':
215252
r = sp.dml(df, y=y, treatment=treatment,
216253
covariates=covariates[:20])

src/statspai/smart/sensitivity.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,18 @@ def sensitivity_dashboard(
124124
'significant': abs(baseline_est / baseline_se) > z_crit if baseline_se > 0 else False,
125125
}
126126

127-
method = getattr(result, 'model_info', {}).get('model_type', 'Unknown')
127+
# Resolve a human-readable method label. Sprint-B CausalResult
128+
# objects store the label on ``.method`` (e.g. "Proximal Causal
129+
# Inference (linear 2SLS)"); older EconometricResults use
130+
# ``model_info['model_type']``. Read both so the dashboard
131+
# doesn't show "Unknown" on proximal / msm / g_computation / etc.
132+
_model_info = getattr(result, 'model_info', {}) or {}
133+
method = (
134+
str(getattr(result, 'method', '') or '')
135+
or str(_model_info.get('model_type', '') or '')
136+
or str(_model_info.get('estimator', '') or '')
137+
or 'Unknown'
138+
)
128139
dim_results = []
129140

130141
if dimensions is None:
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
"""
2+
Round-3 Smart-tool coverage for Sprint-B: compare_estimators registers
3+
g_computation, sensitivity_dashboard reports the correct method label
4+
for Sprint-B results, and ROADMAP §6 documents the compare_estimators
5+
gap for the remaining hint-driven estimators.
6+
"""
7+
8+
from pathlib import Path
9+
10+
import numpy as np
11+
import pandas as pd
12+
import pytest
13+
14+
import statspai as sp
15+
16+
17+
# ---------------------------------------------------------------------
18+
# compare_estimators — g_computation is now a first-class option
19+
# ---------------------------------------------------------------------
20+
21+
@pytest.fixture
22+
def binary_dgp():
23+
rng = np.random.default_rng(0)
24+
n = 500
25+
X = rng.normal(0, 1, n)
26+
D = rng.binomial(1, 0.5, n).astype(float)
27+
Y = 1.0 * D + 0.5 * X + rng.normal(0, 0.3, n)
28+
return pd.DataFrame({'y': Y, 'd': D, 'x': X})
29+
30+
31+
def test_compare_estimators_accepts_g_computation(binary_dgp):
32+
comp = sp.compare_estimators(
33+
data=binary_dgp, y='y', treatment='d',
34+
methods=['ols', 'g_computation'],
35+
covariates=['x'],
36+
)
37+
# Both methods ran (agreement diagnostics populated)
38+
assert hasattr(comp, 'summary')
39+
txt = comp.summary()
40+
assert 'G-computation' in txt
41+
assert 'OLS' in txt
42+
# Sign agreement 100 % on a linear, binary-D, positive-effect DGP.
43+
assert 'Sign agreement: 100%' in txt
44+
45+
46+
def test_compare_estimators_g_computation_recovers_truth(binary_dgp):
47+
"""Both OLS and g_computation should hug the true effect ≈ 1.0."""
48+
comp = sp.compare_estimators(
49+
data=binary_dgp, y='y', treatment='d',
50+
methods=['g_computation'],
51+
covariates=['x'],
52+
)
53+
r = comp.results['G-computation']
54+
assert abs(r.estimate - 1.0) < 0.15
55+
56+
57+
def test_compare_estimators_unsupported_method_does_not_crash(binary_dgp):
58+
"""
59+
Sprint-B methods with extra required args (proximal, msm, ...) are
60+
documented as NOT supported. Requesting one must not silently
61+
drop the supported methods too.
62+
63+
``compare_estimators`` wraps each method in its own try/except +
64+
``warnings.warn`` (see src/statspai/smart/compare.py), so the call
65+
itself MUST NOT raise to the caller. The contract we're pinning:
66+
the supported method (OLS) still appears in ``.results`` even
67+
when an unsupported method ('proximal') sits beside it in the
68+
``methods=`` list.
69+
"""
70+
import warnings
71+
with warnings.catch_warnings():
72+
# compare_estimators emits UserWarning for the unsupported
73+
# branch; suppress it so the test output is clean while still
74+
# exercising the real path.
75+
warnings.simplefilter('ignore')
76+
comp = sp.compare_estimators(
77+
data=binary_dgp, y='y', treatment='d',
78+
methods=['ols', 'proximal'], # 'proximal' unsupported
79+
covariates=['x'],
80+
)
81+
# OLS must still be present regardless of what the unsupported
82+
# method did. No permissive try/except — a true regression
83+
# (OLS silently disappearing) should fail this assertion.
84+
assert 'OLS' in comp.results
85+
# 'proximal' must NOT appear (never dispatched)
86+
assert 'Proximal' not in comp.results
87+
88+
89+
# ---------------------------------------------------------------------
90+
# sensitivity_dashboard — method label now reads from .method first
91+
# ---------------------------------------------------------------------
92+
93+
def test_sensitivity_dashboard_reports_proximal_method_label():
94+
rng = np.random.default_rng(0)
95+
n = 500
96+
U = rng.normal(0, 1, n)
97+
D = 0.8 * U + rng.normal(0, 0.5, n)
98+
Z = 0.9 * U + rng.normal(0, 0.3, n)
99+
W = 0.9 * U + rng.normal(0, 0.3, n)
100+
Y = 1.5 * D + 0.7 * U + rng.normal(0, 0.3, n)
101+
df = pd.DataFrame({'y': Y, 'd': D, 'z': Z, 'w': W})
102+
r = sp.proximal(df, y='y', treat='d', proxy_z=['z'], proxy_w=['w'])
103+
dash = sp.sensitivity_dashboard(r, data=df, verbose=False,
104+
dimensions=['sample'])
105+
assert 'Proximal' in dash.method, (
106+
f"Expected 'Proximal' in dash.method, got {dash.method!r}"
107+
)
108+
109+
110+
def test_sensitivity_dashboard_reports_msm_method_label():
111+
rng = np.random.default_rng(0)
112+
rows = []
113+
for i in range(100):
114+
for t in range(3):
115+
rows.append({
116+
'id': i, 'time': t,
117+
'A': rng.binomial(1, 0.5),
118+
'L': rng.normal(),
119+
'V': rng.normal(),
120+
})
121+
panel = pd.DataFrame(rows)
122+
panel['Y'] = (0.5 * panel.groupby('id')['A'].cumsum()
123+
+ 0.3 * panel['V']
124+
+ rng.normal(0, 0.3, len(panel)))
125+
r = sp.msm(panel, y='Y', treat='A', id='id', time='time',
126+
time_varying=['L'], baseline=['V'])
127+
dash = sp.sensitivity_dashboard(r, data=panel, verbose=False,
128+
dimensions=['sample'])
129+
assert 'Marginal Structural' in dash.method, (
130+
f"Expected 'Marginal Structural' in label, got {dash.method!r}"
131+
)
132+
133+
134+
def test_sensitivity_dashboard_legacy_econometric_result_label():
135+
"""Backward-compat: EconometricResults path still reads model_type."""
136+
rng = np.random.default_rng(0)
137+
n = 200
138+
df = pd.DataFrame({
139+
'y': rng.normal(0, 1, n),
140+
'x': rng.normal(0, 1, n),
141+
})
142+
r = sp.regress('y ~ x', data=df)
143+
dash = sp.sensitivity_dashboard(r, data=df, verbose=False,
144+
dimensions=['sample'])
145+
# Not Unknown — the model_info['model_type'] fallback still works
146+
assert dash.method != 'Unknown'
147+
148+
149+
# ---------------------------------------------------------------------
150+
# ROADMAP §6 documents the compare_estimators Sprint-B gap
151+
# ---------------------------------------------------------------------
152+
153+
def test_compare_estimators_g_computation_binary_guard_compact():
154+
"""
155+
Review-fix: the g_computation branch emits a CLEAN warning on
156+
non-binary D (≤ 500 chars) rather than dumping the full list of
157+
unique D values (which for n=400 continuous D was ~20 KB).
158+
"""
159+
import warnings
160+
rng = np.random.default_rng(0)
161+
n = 400
162+
df = pd.DataFrame({
163+
'y': rng.normal(0, 1, n),
164+
'd': rng.normal(0, 1, n), # continuous — not binary
165+
'x': rng.normal(0, 1, n),
166+
})
167+
with warnings.catch_warnings(record=True) as caught:
168+
warnings.simplefilter('always')
169+
comp = sp.compare_estimators(
170+
data=df, y='y', treatment='d',
171+
methods=['g_computation'],
172+
covariates=['x'],
173+
)
174+
# g_computation branch should have emitted a compact warning.
175+
g_warns = [w for w in caught if 'g_computation' in str(w.message)]
176+
assert len(g_warns) >= 1, "expected a g_computation warning"
177+
msg = str(g_warns[0].message)
178+
# Compact: the warning fits on a few lines, not a 20 KB value dump
179+
assert len(msg) < 500, (
180+
f"g_computation warning is {len(msg)} chars; expected <500. "
181+
f"First 200 chars: {msg[:200]}"
182+
)
183+
# Useful: it actually mentions the issue and points to the fix
184+
assert 'binary' in msg.lower()
185+
186+
187+
def test_roadmap_documents_compare_estimators_sprint_b_gap():
188+
root = Path(__file__).resolve().parent.parent
189+
roadmap = (root / 'docs' / 'ROADMAP.md').read_text()
190+
# Section 6 heading present
191+
assert 'compare_estimators' in roadmap
192+
# The four hint-driven methods are called out
193+
for name in ('proximal', 'msm', 'principal_strat', 'mediator'):
194+
assert name in roadmap.lower()

0 commit comments

Comments
 (0)