Address PR igerber#192 review (Round 7): fix REGISTRY labels, remove unused params, add inline comments

igerber · claude · igerber · commit 471b1ea26750 · 2026-03-14T15:59:10.000-04:00
Relabel REGISTRY.md PT-All and bootstrap entries with **Note:** prefix so
the review prompt's deviation-detection logic recognizes them as documented
choices. Remove unused parameters (target_t from enumerate_valid_triples,
att_gt from compute_eif_nocov) and all call sites. Expand inline comments
at flagged code locations explaining g'=∞ telescoping and period_1
degenerate-term exclusion.

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/diff_diff/efficient_did.py b/diff_diff/efficient_did.py
@@ -333,7 +333,6 @@ def fit(
                 # Enumerate valid comparison pairs
                 pairs = enumerate_valid_triples(
                     target_g=g,
-                    target_t=t,
                     treatment_groups=treatment_groups,
                     time_periods=time_periods,
                     period_1=period_1,
@@ -398,7 +397,6 @@ def fit(
                 eif_vals = compute_eif_nocov(
                     target_g=g,
                     target_t=t,
-                    att_gt=att_gt,
                     weights=weights,
                     valid_pairs=pairs,
                     outcome_wide=outcome_wide,
diff --git a/diff_diff/efficient_did_weights.py b/diff_diff/efficient_did_weights.py
@@ -17,7 +17,6 @@
 
 def enumerate_valid_triples(
     target_g: float,
-    target_t: float,
     treatment_groups: List[float],
     time_periods: List[float],
     period_1: float,
@@ -39,8 +38,6 @@ def enumerate_valid_triples(
     ----------
     target_g : float
         Treatment cohort of the target group.
-    target_t : float
-        Time period of the target parameter.
     treatment_groups : list of float
         All treatment cohort identifiers (finite values only).
     time_periods : list of float
@@ -69,9 +66,13 @@ def enumerate_valid_triples(
     # PT-All: overidentified
     pairs: List[Tuple[float, float]] = []
 
-    # Candidate comparison groups: never-treated + all treatment cohorts
-    # (including g'=g — same-cohort pairs are valid under PT-All and
-    # contribute overidentifying moments; see Eq 3.9).
+    # Candidate comparison groups: never-treated + all treatment cohorts.
+    # Including g'=g (same-cohort) is valid under PT-All (Eq 3.9).
+    # Including g'=∞ (never-treated) produces moments where the second
+    # and third terms telescope: y_hat = E[Y_t-Y_1|G=g] - E[Y_t-Y_1|G=∞]
+    # regardless of t_pre. These redundant moments add no information
+    # beyond the basic 2x2 DiD; Omega*'s pseudoinverse assigns them
+    # zero effective weight. Retained for implementation simplicity.
     candidate_groups: List[float] = [never_treated_val]
     for gp in treatment_groups:
         candidate_groups.append(gp)
@@ -85,8 +86,10 @@ def enumerate_valid_triples(
 
         for t_pre in time_periods:
             if t_pre == period_1:
-                # period_1 is the universal reference — used as Y_1 in
-                # differencing, not as a selectable baseline t_pre
+                # period_1 is the universal reference — used as Y_1 in the
+                # differencing (Eq 3.9 first term). Including t_pre = period_1
+                # would make the third term Y_1 - Y_1 = 0 (degenerate), so it
+                # adds no information to Omega* regardless of which g' is used.
                 continue
             # Only require t_pre < g' (pre-treatment for comparison group).
             # No constraint on t_pre vs g: the target group appears only in
@@ -419,7 +422,6 @@ def compute_generated_outcomes_nocov(
 def compute_eif_nocov(
     target_g: float,
     target_t: float,
-    att_gt: float,
     weights: np.ndarray,
     valid_pairs: List[Tuple[float, float]],
     outcome_wide: np.ndarray,
@@ -454,8 +456,6 @@ def compute_eif_nocov(
     ----------
     target_g, target_t : float
         Target group-time.
-    att_gt : float
-        Estimated ATT(g, t).
     weights : ndarray, shape (H,)
         Efficient weights.
     valid_pairs : list of (g', t_pre)
diff --git a/docs/methodology/REGISTRY.md b/docs/methodology/REGISTRY.md
@@ -587,8 +587,8 @@ where `q_{g,e} = pi_g / sum_{g' in G_{trt,e}} pi_{g'}`.
 - **Negative weights**: Explicitly stated as harmless for bias and beneficial for precision; arise from efficiency optimization under overidentification (Section 5.2)
 - **PT-Post regime (just-identified)**: Under PT-Post, EDiD automatically reduces to standard single-baseline estimator (Corollary 3.2). No downside to using EDiD -- it subsumes standard estimators
 - **Duplicate rows**: Duplicate `(unit, time)` entries are rejected with `ValueError`. The estimator requires exactly one observation per unit-period
-- **PT-All index set**: Under PT-All, valid (g', t_pre) pairs require only t_pre < g' (pre-treatment for the comparison group), not t_pre < g. Same-group pairs (g'=g) are valid and contribute overidentifying moments. This follows from Equation 3.9: the target group g appears only in the first term (Y_t - Y_1), which is independent of t_pre
-- **Bootstrap aggregation**: Multiplier bootstrap uses fixed cohort-size weights for overall/event-study aggregation, matching the CallawaySantAnna bootstrap pattern (CallawaySantAnnaBootstrapMixin._run_multiplier_bootstrap). The analytical path includes a WIF correction; the bootstrap captures sampling variability through per-cell EIF perturbation without re-estimating aggregation weights, consistent with both the library's CS implementation and the R `did` package approach
+- **Note:** PT-All index set includes g'=∞ (never-treated) as a candidate comparison group and excludes period_1 for all g'. When g'=∞, the second and third Eq 3.9 terms telescope so all (∞, t_pre) moments produce the same 2x2 DiD value; these redundant moments are handled by Omega*'s pseudoinverse. When t_pre = period_1, the third term degenerates to E[Y_1 - Y_1 | G=g'] = 0 for any g', adding no information. Valid pairs require only t_pre < g' (pre-treatment for comparison group), not t_pre < g. Same-group pairs (g'=g) are valid and contribute overidentifying moments (Equation 3.9).
+- **Note:** Bootstrap aggregation uses fixed cohort-size weights for overall/event-study reaggregation, matching the CallawaySantAnna bootstrap pattern (staggered_bootstrap.py:281 computes `bootstrap_overall = bootstrap_atts_gt[:, post_indices] @ weights`; L297 uses the same fixed-weight pattern for event study). The analytical path includes a WIF correction; fixed-weight bootstrap captures the same sampling variability through per-cell EIF perturbation without re-estimating aggregation weights, consistent with both the library's CS implementation and the R `did` package.
 - **Overall ATT convention**: The library's `overall_att` uses cohort-size-weighted averaging of post-treatment (g,t) cells, matching the CallawaySantAnna simple aggregation. This differs from the paper's ES_avg (Eq 2.3), which uniformly averages over event-time horizons. ES_avg can be computed from event study output as `mean(event_study_effects[e]["effect"] for e >= 0)`
 
 *Algorithm (two-step semiparametric estimation, Section 4):*
diff --git a/tests/test_efficient_did.py b/tests/test_efficient_did.py
@@ -540,7 +540,6 @@ def test_pt_all_simple(self):
         Total: 5 pairs."""
         pairs = enumerate_valid_triples(
             target_g=3,
-            target_t=4,
             treatment_groups=[3],
             time_periods=[1, 2, 3, 4, 5],
             period_1=1,
@@ -558,7 +557,6 @@ def test_pt_all_staggered(self):
         Total: 8 pairs."""
         pairs = enumerate_valid_triples(
             target_g=3,
-            target_t=4,
             treatment_groups=[3, 5],
             time_periods=[1, 2, 3, 4, 5],
             period_1=1,
@@ -581,7 +579,6 @@ def test_pt_post_single_pair(self):
         """PT-Post: only (inf, g-1)."""
         pairs = enumerate_valid_triples(
             target_g=3,
-            target_t=4,
             treatment_groups=[3, 5],
             time_periods=[1, 2, 3, 4, 5],
             period_1=1,
@@ -595,7 +592,6 @@ def test_g2_has_valid_pairs_pt_all(self):
         So pairs should be non-empty."""
         pairs = enumerate_valid_triples(
             target_g=2,
-            target_t=3,
             treatment_groups=[2],
             time_periods=[1, 2, 3],
             period_1=1,
@@ -611,7 +607,6 @@ def test_anticipation(self):
         """Anticipation shifts effective treatment boundary."""
         pairs_no_ant = enumerate_valid_triples(
             target_g=4,
-            target_t=5,
             treatment_groups=[4],
             time_periods=[1, 2, 3, 4, 5],
             period_1=1,
@@ -620,7 +615,6 @@ def test_anticipation(self):
         )
         pairs_ant1 = enumerate_valid_triples(
             target_g=4,
-            target_t=5,
             treatment_groups=[4],
             time_periods=[1, 2, 3, 4, 5],
             period_1=1,
@@ -1051,15 +1045,13 @@ def test_pt_all_more_moments_than_pt_post(self):
         """PT-All should produce strictly more moments than PT-Post."""
         pairs_all = enumerate_valid_triples(
             target_g=3,
-            target_t=4,
             treatment_groups=[3, 5],
             time_periods=[1, 2, 3, 4, 5, 6],
             period_1=1,
             pt_assumption="all",
         )
         pairs_post = enumerate_valid_triples(
             target_g=3,
-            target_t=4,
             treatment_groups=[3, 5],
             time_periods=[1, 2, 3, 4, 5, 6],
             period_1=1,
@@ -1074,7 +1066,6 @@ def test_same_group_pairs_valid(self):
         """g'=g pairs should be present in PT-All enumeration."""
         pairs = enumerate_valid_triples(
             target_g=3,
-            target_t=4,
             treatment_groups=[3, 5],
             time_periods=[1, 2, 3, 4, 5],
             period_1=1,
