Skip to content

Commit 3c89a4c

Browse files
igerberclaude
andcommitted
Remove dead Rust compute_synthetic_weights (post-audit cleanup for finding #22)
Closes the Rust-side loop on finding #22 from the silent-failures audit. The Python-side wrapper and its sole caller were removed in PR #344; this PR deletes the now-orphaned PyO3 binding, its internal PGD implementation, and the obsolete constants that only serviced it. Deleted - rust/src/weights.rs: compute_synthetic_weights pyfunction + compute_synthetic_weights_internal - rust/src/weights.rs: MAX_ITER, DEFAULT_TOL, DEFAULT_STEP_SIZE (only used by the deleted PGD path) - rust/src/weights.rs: test_compute_weights_sum_to_one (cargo test for the deleted internal helper) - rust/src/lib.rs: m.add_function(wrap_pyfunction!(weights::compute_synthetic_weights, m)?) Retained (still used from Python) - weights::project_simplex — consumed by _rust_project_simplex in utils.py (project_simplex tests in test_rust_backend.py) - weights::compute_sdid_unit_weights, compute_time_weights, sc_weight_fw, compute_noise_level (SDID canonical path) - weights::sum_normalize_internal, sparsify_internal (helpers for SDID) Verification - maturin develop --release --features accelerate: clean rebuild, no warnings - cargo check --lib: clean (cargo test --release hits a known macOS PyO3 Python3.framework rpath issue unrelated to this change) - pytest tests/test_rust_backend.py tests/test_prep.py::TestRankControlUnits: 85 passed - Direct import test: project_simplex / compute_sdid_unit_weights / compute_time_weights / sc_weight_fw still importable; compute_synthetic_weights correctly raises ImportError TODO.md row 86 removed. docs/audits/silent-failures-findings.md row #22 updated to FULLY RESOLVED. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9146f1e commit 3c89a4c

3 files changed

Lines changed: 1 addition & 126 deletions

File tree

TODO.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ Deferred items from PR reviews that were not addressed before merge.
8383
| Weighted CR2 Bell-McCaffrey cluster-robust (`vcov_type="hc2_bm"` + `cluster_ids` + `weights`) currently raises `NotImplementedError`. Weighted hat matrix and residual rebalancing need threading per clubSandwich WLS handling. | `linalg.py::_compute_cr2_bm` | Phase 1a | Medium |
8484
| Regenerate `benchmarks/data/clubsandwich_cr2_golden.json` from R (`Rscript benchmarks/R/generate_clubsandwich_golden.R`). Current JSON has `source: python_self_reference` as a stability anchor until an authoritative R run. | `benchmarks/R/generate_clubsandwich_golden.R` | Phase 1a | Medium |
8585
| `honest_did.py:1907` `np.linalg.solve(A_sys, b_sys) / except LinAlgError: continue` is a silent basis-rejection in the vertex-enumeration loop that is algorithmically intentional (try the next basis). Consider surfacing a count of rejected bases as a diagnostic when ARP enumeration exhausts, so users see when the vertex search was heavily constrained. Not a silent failure in the sense of the Phase 2 audit (the algorithm is supposed to skip), but the diagnostic would help debug borderline cases. | `honest_did.py` | #334 | Low |
86-
| Rust `compute_synthetic_weights` + `compute_synthetic_weights_internal` (now dead code) can be removed from `rust/src/weights.rs:43-117` in a future Rust-cleanup PR. Python-side wrapper was deleted (post-audit cleanup for finding #22) and its sole caller now inlines Frank-Wolfe via `_sc_weight_fw`. The Rust symbol remains callable via `from diff_diff._rust_backend import compute_synthetic_weights` but no Python code calls it. Removal requires `maturin develop` rebuild. No functional impact of leaving it. | `rust/src/weights.rs` | follow-up | Low |
8786
| TROP Rust vs Python grid-search divergence on rank-deficient Y: on two near-parallel control units, LOOCV grid-search ATT diverges ~6% between Rust (`trop_global.py:688`) and Python fallback (`trop_global.py:753`). Either grid-winner ties are broken differently or the per-λ solver reaches different stationary points under rank deficiency. Audit finding #23 flagged this surface. `@pytest.mark.xfail(strict=True)` in `tests/test_rust_backend.py::TestTROPRustEdgeCaseParity::test_grid_search_rank_deficient_Y` baselines the gap. | `trop_global.py`, `rust/` | follow-up | Medium |
8887
| TROP Rust vs Python bootstrap SE divergence under fixed seed: `seed=42` on a tiny panel produces ~28% bootstrap-SE gap. Root cause: Rust bootstrap uses its own RNG (`rand` crate) while Python uses `numpy.random.default_rng`; same seed value maps to different bytestreams across backends. Audit axis-H (RNG/seed) adjacent. `@pytest.mark.xfail(strict=True)` in `tests/test_rust_backend.py::TestTROPRustEdgeCaseParity::test_bootstrap_seed_reproducibility` baselines the gap. Unifying RNG (threading a numpy-generated seed-sequence into Rust, or porting Python to ChaCha) would close it. | `trop_global.py`, `rust/` | follow-up | Medium |
8988
| `bias_corrected_local_linear`: extend golden parity to `kernel="triangular"` and `kernel="uniform"` (currently epa-only; all three kernels share `kernel_W` and the `lprobust` math, so parity is expected but not separately asserted). | `benchmarks/R/generate_nprobust_lprobust_golden.R`, `tests/test_bias_corrected_lprobust.py` | Phase 1c | Low |

rust/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ fn _rust_backend(m: &Bound<'_, PyModule>) -> PyResult<()> {
2828
m
2929
)?)?;
3030

31-
// Synthetic control weights (legacy projected gradient descent)
32-
m.add_function(wrap_pyfunction!(weights::compute_synthetic_weights, m)?)?;
31+
// Simplex projection
3332
m.add_function(wrap_pyfunction!(weights::project_simplex, m)?)?;
3433

3534
// SDID weights (Frank-Wolfe matching R's synthdid)

rust/src/weights.rs

Lines changed: 0 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Synthetic control weight computation.
22
//!
33
//! This module provides optimized implementations of:
4-
//! - Legacy synthetic control weight optimization (projected gradient descent)
54
//! - Frank-Wolfe synthetic control weights (matching R's synthdid)
65
//! - Simplex projection
76
//! - SDID unit and time weight computation
@@ -11,111 +10,6 @@ use ndarray::linalg::general_mat_vec_mul;
1110
use numpy::{PyArray1, PyReadonlyArray1, PyReadonlyArray2, ToPyArray};
1211
use pyo3::prelude::*;
1312

14-
/// Maximum number of optimization iterations.
15-
const MAX_ITER: usize = 1000;
16-
17-
/// Default convergence tolerance (matches Python's _OPTIMIZATION_TOL).
18-
const DEFAULT_TOL: f64 = 1e-8;
19-
20-
/// Default step size for gradient descent.
21-
const DEFAULT_STEP_SIZE: f64 = 0.1;
22-
23-
// =========================================================================
24-
// Legacy synthetic control weights (projected gradient descent)
25-
// =========================================================================
26-
27-
/// Compute synthetic control weights via projected gradient descent.
28-
///
29-
/// Solves: min_w ||Y_treated - Y_control @ w||² + lambda * ||w||²
30-
/// subject to: w >= 0, sum(w) = 1
31-
///
32-
/// # Arguments
33-
/// * `y_control` - Control unit outcomes matrix (n_pre, n_control)
34-
/// * `y_treated` - Treated unit outcomes (n_pre,)
35-
/// * `lambda_reg` - L2 regularization parameter
36-
/// * `max_iter` - Maximum number of iterations (default: 1000)
37-
/// * `tol` - Convergence tolerance (default: 1e-6)
38-
///
39-
/// # Returns
40-
/// Optimal weights (n_control,) that sum to 1
41-
#[pyfunction]
42-
#[pyo3(signature = (y_control, y_treated, lambda_reg=0.0, max_iter=None, tol=None))]
43-
pub fn compute_synthetic_weights<'py>(
44-
py: Python<'py>,
45-
y_control: PyReadonlyArray2<'py, f64>,
46-
y_treated: PyReadonlyArray1<'py, f64>,
47-
lambda_reg: f64,
48-
max_iter: Option<usize>,
49-
tol: Option<f64>,
50-
) -> PyResult<Bound<'py, PyArray1<f64>>> {
51-
let y_control_arr = y_control.as_array();
52-
let y_treated_arr = y_treated.as_array();
53-
54-
let weights =
55-
compute_synthetic_weights_internal(&y_control_arr, &y_treated_arr, lambda_reg, max_iter, tol)?;
56-
57-
Ok(weights.to_pyarray(py))
58-
}
59-
60-
/// Internal implementation of synthetic weight computation.
61-
fn compute_synthetic_weights_internal(
62-
y_control: &ArrayView2<f64>,
63-
y_treated: &ArrayView1<f64>,
64-
lambda_reg: f64,
65-
max_iter: Option<usize>,
66-
tol: Option<f64>,
67-
) -> PyResult<Array1<f64>> {
68-
let n_control = y_control.ncols();
69-
let max_iter = max_iter.unwrap_or(MAX_ITER);
70-
let tol = tol.unwrap_or(DEFAULT_TOL);
71-
72-
// Precompute Hessian: H = Y_control' @ Y_control + lambda * I
73-
let h = {
74-
let ytc = y_control.t().dot(y_control);
75-
let mut h = ytc;
76-
// Add regularization to diagonal
77-
for i in 0..n_control {
78-
h[[i, i]] += lambda_reg;
79-
}
80-
h
81-
};
82-
83-
// Precompute linear term: f = Y_control' @ Y_treated
84-
let f = y_control.t().dot(y_treated);
85-
86-
// Initialize with uniform weights
87-
let mut weights = Array1::from_elem(n_control, 1.0 / n_control as f64);
88-
89-
// Projected gradient descent
90-
let step_size = DEFAULT_STEP_SIZE;
91-
let mut prev_weights = weights.clone();
92-
93-
for _ in 0..max_iter {
94-
// Gradient: grad = H @ weights - f
95-
let grad = h.dot(&weights) - &f;
96-
97-
// Gradient step
98-
weights = &weights - step_size * &grad;
99-
100-
// Project onto simplex
101-
weights = project_simplex_internal(&weights.view());
102-
103-
// Check convergence
104-
let diff: f64 = weights
105-
.iter()
106-
.zip(prev_weights.iter())
107-
.map(|(a, b)| (a - b).powi(2))
108-
.sum();
109-
if diff.sqrt() < tol {
110-
break;
111-
}
112-
113-
prev_weights.assign(&weights);
114-
}
115-
116-
Ok(weights)
117-
}
118-
11913
// =========================================================================
12014
// Simplex projection
12115
// =========================================================================
@@ -806,23 +700,6 @@ mod tests {
806700
assert!(result.iter().all(|&x| x >= -1e-10));
807701
}
808702

809-
#[test]
810-
fn test_compute_weights_sum_to_one() {
811-
let y_control = array![[1.0, 2.0, 3.0], [4.0, 5.0, 6.0], [7.0, 8.0, 9.0]];
812-
let y_treated = array![2.0, 5.0, 8.0];
813-
814-
let weights =
815-
compute_synthetic_weights_internal(&y_control.view(), &y_treated.view(), 0.0, None, None)
816-
.unwrap();
817-
818-
let sum: f64 = weights.sum();
819-
assert!((sum - 1.0).abs() < 1e-6, "Weights should sum to 1, got {}", sum);
820-
assert!(
821-
weights.iter().all(|&w| w >= -1e-10),
822-
"Weights should be non-negative"
823-
);
824-
}
825-
826703
#[test]
827704
fn test_sum_normalize() {
828705
let v = array![2.0, 3.0, 5.0];

0 commit comments

Comments
 (0)