Skip to content

Commit 78a6d08

Browse files
fix(bartik): proper leave-one-out with regional_shocks panel + fail-loud warning
Pre-fix, `bartik(leave_one_out=True)` (the default!) silently fell through to the simple Bartik instrument because proper LOO requires per-region industry growth data that the basic API didn't carry. The docstring promised "exclude own region from national average" but the implementation was `pass`. Users relying on LOO got lied to. Fix: - Add optional `regional_shocks: pd.DataFrame` (n_units x n_industries) parameter. When provided with leave_one_out=True, compute exact Borusyak-Hull-Jaravel 2022 LOO: g_k^{-i} = (sum_j g_{jk} - g_{ik})/(n-1), then B_i = sum_k s_ik * g_k^{-i}. - When leave_one_out=True but regional_shocks is missing, emit a UserWarning explaining the fallback and remediation — silent bug becomes loud. - Validate regional_shocks shape and industry coverage upfront. Tests: TestBartikLeaveOneOut (7 cases) covers warn-on-missing, silent-when-disabled, panel activates LOO, LOO ≠ simple Bartik, exact-formula match vs. manual computation (rtol=1e-10), and two validation errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a8fbd1f commit 78a6d08

2 files changed

Lines changed: 184 additions & 7 deletions

File tree

src/statspai/bartik/shift_share.py

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
"""
1717

1818
from typing import Optional, List, Dict, Any
19+
import warnings
20+
1921
import numpy as np
2022
import pandas as pd
2123
from scipy import stats
@@ -31,6 +33,7 @@ def bartik(
3133
shocks: pd.Series,
3234
covariates: Optional[List[str]] = None,
3335
leave_one_out: bool = True,
36+
regional_shocks: Optional[pd.DataFrame] = None,
3437
robust: str = 'hc1',
3538
alpha: float = 0.05,
3639
) -> EconometricResults:
@@ -53,7 +56,23 @@ def bartik(
5356
covariates : list of str, optional
5457
Exogenous control variables.
5558
leave_one_out : bool, default True
56-
Compute leave-one-out shocks (exclude own region from national average).
59+
Compute leave-one-out shocks (exclude own region from national
60+
average). Only takes effect when ``regional_shocks`` is also
61+
supplied — without the per-region industry growth panel there
62+
is not enough information to reconstruct ``g_k`` excluding
63+
region ``i``. When ``leave_one_out=True`` but
64+
``regional_shocks`` is not provided, a ``UserWarning`` is
65+
raised and the estimator falls back to the simple Bartik
66+
instrument.
67+
regional_shocks : pd.DataFrame, optional
68+
Regional industry growth matrix (n_units x n_industries). Row
69+
``i``, column ``k`` is the realised growth of industry ``k``
70+
in region ``i``. When provided with ``leave_one_out=True``,
71+
the instrument uses
72+
``g_k^{-i} = (sum_j g_{jk} - g_{ik}) / (n - 1)`` (Borusyak-
73+
Hull-Jaravel 2022-style exact leave-one-out). Row index must
74+
align with ``shares``; columns must be a superset of
75+
``shocks.index``.
5776
robust : str, default 'hc1'
5877
Standard error type.
5978
alpha : float, default 0.05
@@ -74,6 +93,7 @@ def bartik(
7493
estimator = BartikIV(
7594
data=data, y=y, endog=endog, shares=shares, shocks=shocks,
7695
covariates=covariates, leave_one_out=leave_one_out,
96+
regional_shocks=regional_shocks,
7797
robust=robust, alpha=alpha,
7898
)
7999
return estimator.fit()
@@ -93,6 +113,7 @@ def __init__(
93113
shocks: pd.Series,
94114
covariates: Optional[List[str]] = None,
95115
leave_one_out: bool = True,
116+
regional_shocks: Optional[pd.DataFrame] = None,
96117
robust: str = 'hc1',
97118
alpha: float = 0.05,
98119
):
@@ -103,6 +124,7 @@ def __init__(
103124
self.shocks = shocks
104125
self.covariates = covariates or []
105126
self.leave_one_out = leave_one_out
127+
self.regional_shocks = regional_shocks
106128
self.robust = robust
107129
self.alpha = alpha
108130

@@ -127,17 +149,66 @@ def _validate(self):
127149
self.shares = self.shares[common]
128150
self.shocks = self.shocks[common]
129151

152+
if self.regional_shocks is not None:
153+
if self.regional_shocks.shape[0] != len(self.data):
154+
raise ValueError(
155+
f"regional_shocks has {self.regional_shocks.shape[0]} "
156+
f"rows but data has {len(self.data)} rows"
157+
)
158+
missing = set(common) - set(self.regional_shocks.columns)
159+
if missing:
160+
raise ValueError(
161+
"regional_shocks is missing industries present in "
162+
f"shares/shocks: {sorted(missing)}"
163+
)
164+
self.regional_shocks = self.regional_shocks[common]
165+
130166
def _construct_instrument(self) -> np.ndarray:
131-
"""Construct the Bartik instrument: B_i = sum_k(s_ik * g_k)."""
167+
"""Construct the Bartik instrument.
168+
169+
- ``B_i = sum_k s_ik * g_k`` (simple Bartik), or
170+
- ``B_i = sum_k s_ik * g_k^{-i}`` (leave-one-out) when
171+
``leave_one_out=True`` and ``regional_shocks`` is provided,
172+
where ``g_k^{-i} = (sum_j g_{jk} - g_{ik}) / (n - 1)`` is
173+
the national industry growth rate computed excluding
174+
region ``i`` (Borusyak-Hull-Jaravel 2022).
175+
176+
Silent-no-op guard: prior to v0.9.13 ``leave_one_out=True``
177+
quietly fell through to simple Bartik because the LOO step
178+
requires per-region industry growth data that the basic API
179+
does not carry. We now warn loudly instead so users notice
180+
the fallback.
181+
"""
132182
S = self.shares.values # (n, K)
133183
g = self.shocks.values # (K,)
134184

135-
if self.leave_one_out:
136-
# TODO: proper leave-one-out requires additional data structure
137-
# For now, use simple Bartik
138-
pass
185+
if not self.leave_one_out:
186+
return S @ g # (n,)
187+
188+
if self.regional_shocks is None:
189+
warnings.warn(
190+
"bartik(leave_one_out=True) requested but "
191+
"`regional_shocks` was not supplied. Proper "
192+
"leave-one-out requires per-region industry growth "
193+
"(n_units x n_industries) to reconstruct g_k^{-i}. "
194+
"Falling back to the simple Bartik instrument "
195+
"B_i = sum_k s_ik * g_k; pass `regional_shocks=` or "
196+
"set `leave_one_out=False` to silence this warning.",
197+
UserWarning,
198+
stacklevel=3,
199+
)
200+
return S @ g
139201

140-
return S @ g # (n,)
202+
G = self.regional_shocks.values.astype(float) # (n, K)
203+
n = G.shape[0]
204+
if n < 2:
205+
raise ValueError(
206+
"leave-one-out requires at least 2 regions"
207+
)
208+
# g_k^{-i} = (col_sum_k - G[i,k]) / (n - 1)
209+
col_sum = G.sum(axis=0, keepdims=True) # (1, K)
210+
g_loo = (col_sum - G) / (n - 1) # (n, K)
211+
return np.einsum('ij,ij->i', S, g_loo) # (n,)
141212

142213
def _rotemberg_weights(self, B, Y, X_endog, X_exog):
143214
"""

tests/test_bartik.py

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
Tests for Shift-Share (Bartik) IV module.
33
"""
44

5+
import warnings
6+
57
import pytest
68
import numpy as np
79
import pandas as pd
@@ -146,5 +148,109 @@ def test_no_common_industries(self, bartik_data):
146148
shares=shares, shocks=bad_shocks)
147149

148150

151+
class TestBartikLeaveOneOut:
152+
"""v0.9.13: proper leave-one-out requires `regional_shocks` panel.
153+
154+
Before v0.9.13 `leave_one_out=True` silently fell through to the
155+
simple Bartik instrument. Now it either computes the true LOO
156+
instrument or warns the user about the fallback.
157+
"""
158+
159+
@pytest.fixture
160+
def bartik_loo_data(self, bartik_data):
161+
"""Attach a regional_shocks panel consistent with bartik_data."""
162+
data, shares, shocks = bartik_data
163+
rng = np.random.default_rng(2026)
164+
# Regional industry growth — national shock + regional noise.
165+
# Mean across regions is close to the aggregated shock but
166+
# leave-one-out must subtract the region's own contribution.
167+
G = shocks.values[None, :] + rng.normal(0, 0.01,
168+
(len(data), len(shocks)))
169+
regional = pd.DataFrame(G, columns=shares.columns)
170+
return data, shares, shocks, regional
171+
172+
def test_loo_warns_when_regional_shocks_missing(self, bartik_data):
173+
data, shares, shocks = bartik_data
174+
with pytest.warns(UserWarning, match="regional_shocks"):
175+
bartik(data, y='y', endog='x_endog',
176+
shares=shares, shocks=shocks, leave_one_out=True)
177+
178+
def test_loo_silent_when_disabled(self, bartik_data):
179+
data, shares, shocks = bartik_data
180+
with warnings.catch_warnings():
181+
warnings.simplefilter('error', UserWarning)
182+
bartik(data, y='y', endog='x_endog',
183+
shares=shares, shocks=shocks, leave_one_out=False)
184+
185+
def test_loo_uses_panel_when_provided(self, bartik_loo_data):
186+
data, shares, shocks, regional = bartik_loo_data
187+
with warnings.catch_warnings():
188+
warnings.simplefilter('error', UserWarning)
189+
# Must NOT warn because regional_shocks covers the LOO path
190+
result = bartik(data, y='y', endog='x_endog',
191+
shares=shares, shocks=shocks,
192+
regional_shocks=regional,
193+
leave_one_out=True)
194+
assert result is not None
195+
196+
def test_loo_instrument_differs_from_simple_bartik(self, bartik_loo_data):
197+
"""B_i^{LOO} should differ from B_i^{simple} when regional
198+
growth differs from the aggregated shock."""
199+
data, shares, shocks, regional = bartik_loo_data
200+
model_simple = BartikIV(data=data, y='y', endog='x_endog',
201+
shares=shares, shocks=shocks,
202+
leave_one_out=False)
203+
model_simple._validate()
204+
B_simple = model_simple._construct_instrument()
205+
206+
model_loo = BartikIV(data=data, y='y', endog='x_endog',
207+
shares=shares, shocks=shocks,
208+
regional_shocks=regional,
209+
leave_one_out=True)
210+
model_loo._validate()
211+
B_loo = model_loo._construct_instrument()
212+
213+
# They should be close (LOO ≈ national when noise is small)
214+
# but not identical.
215+
assert not np.allclose(B_simple, B_loo)
216+
assert np.corrcoef(B_simple, B_loo)[0, 1] > 0.5
217+
218+
def test_loo_formula_matches_manual_computation(self, bartik_loo_data):
219+
"""Verify: g_k^{-i} = (sum_j g_{jk} - g_{ik}) / (n - 1)."""
220+
data, shares, shocks, regional = bartik_loo_data
221+
model = BartikIV(data=data, y='y', endog='x_endog',
222+
shares=shares, shocks=shocks,
223+
regional_shocks=regional,
224+
leave_one_out=True)
225+
model._validate()
226+
B = model._construct_instrument()
227+
228+
G = regional.values
229+
n, K = G.shape
230+
expected = np.zeros(n)
231+
for i in range(n):
232+
for k in range(K):
233+
g_loo_ik = (G[:, k].sum() - G[i, k]) / (n - 1)
234+
expected[i] += shares.values[i, k] * g_loo_ik
235+
np.testing.assert_allclose(B, expected, rtol=1e-10)
236+
237+
def test_regional_shocks_wrong_rows_raises(self, bartik_loo_data):
238+
data, shares, shocks, regional = bartik_loo_data
239+
with pytest.raises(ValueError, match="regional_shocks has"):
240+
bartik(data, y='y', endog='x_endog',
241+
shares=shares, shocks=shocks,
242+
regional_shocks=regional.iloc[:50],
243+
leave_one_out=True)
244+
245+
def test_regional_shocks_missing_industry_raises(self, bartik_loo_data):
246+
data, shares, shocks, regional = bartik_loo_data
247+
partial = regional.drop(columns=['ind_0'])
248+
with pytest.raises(ValueError, match="missing industries"):
249+
bartik(data, y='y', endog='x_endog',
250+
shares=shares, shocks=shocks,
251+
regional_shocks=partial,
252+
leave_one_out=True)
253+
254+
149255
if __name__ == "__main__":
150256
pytest.main([__file__, '-v'])

0 commit comments

Comments
 (0)