Skip to content

Commit 4f66020

Browse files
authored
Merge pull request #155 from eurunuela/perf/batch-univariate-fista
Batch univariate FISTA across voxels (~50×) and drop redundant JAX syncs
2 parents b858b27 + 03429ab commit 4f66020

3 files changed

Lines changed: 74 additions & 36 deletions

File tree

pySPFM/_solvers/fista.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -337,13 +337,16 @@ def fista(
337337
_fista_update_jit = jax.jit(_fista_update)
338338
_has_converged_jit = jax.jit(_has_converged)
339339

340-
# Perform FISTA
340+
# Perform FISTA. The intermediate forward/prox/update steps are left
341+
# async -- JAX pipelines them and the convergence check below forces the
342+
# single sync per iteration that's actually needed. (Calling
343+
# ``block_until_ready`` after every op serialized the loop for no benefit.)
341344
for num_iter in range(max_iter):
342345
# Save results from previous iteration
343346
s_old = s.copy()
344347
y_ista_s = y_fista_s.copy()
345348

346-
z_ista_s = _fista_forward_jit(v, hrf_cov, y_ista_s, c_ist).block_until_ready()
349+
z_ista_s = _fista_forward_jit(v, hrf_cov, y_ista_s, c_ist)
347350

348351
# Per-voxel weighted threshold for the mixed-norm prox. With no
349352
# weights this is exactly ``c_ist * lambda_`` (bit-identical to the
@@ -357,29 +360,26 @@ def fista(
357360
z_regs = z_ista_s[n_scans:]
358361

359362
if group > 0:
360-
s_hrf = proximal_operator_mixed_norm_jit(
361-
z_hrf, mixed_thr, rho_val=(1 - group)
362-
).block_until_ready()
363+
s_hrf = proximal_operator_mixed_norm_jit(z_hrf, mixed_thr, rho_val=(1 - group))
363364
else:
364-
s_hrf = proximal_operator_lasso_jit(z_hrf, c_ist * lambda_).block_until_ready()
365+
s_hrf = proximal_operator_lasso_jit(z_hrf, c_ist * lambda_)
365366

366367
s = jnp.vstack((s_hrf, z_regs))
367368
else:
368369
if group > 0:
369-
s = proximal_operator_mixed_norm_jit(
370-
z_ista_s, mixed_thr, rho_val=(1 - group)
371-
).block_until_ready()
370+
s = proximal_operator_mixed_norm_jit(z_ista_s, mixed_thr, rho_val=(1 - group))
372371
else:
373-
s = proximal_operator_lasso_jit(z_ista_s, c_ist * lambda_).block_until_ready()
372+
s = proximal_operator_lasso_jit(z_ista_s, c_ist * lambda_)
374373

375374
if positive_only:
376375
s = np.sign(hrf[1, 0]) * jnp.maximum(np.sign(hrf[1, 0]) * s, 0)
377376

378377
t_fista, y_fista_s = _fista_update_jit(t_fista, s, s_old)
379378

380-
# Convergence. Pass (current, previous) so _has_converged normalizes
381-
# the change by |s_old| as documented (the args were previously swapped).
382-
if num_iter >= min_iter and _has_converged_jit(s, s_old, tol).block_until_ready():
379+
# Convergence: pass (current, previous) so _has_converged normalizes
380+
# by |s_old| as documented. This is also the single per-iteration sync
381+
# (the forward/prox/update steps above are left async).
382+
if num_iter >= min_iter and bool(_has_converged_jit(s, s_old, tol)):
383383
break
384384

385385
LGR.debug(f"Iteration: {str(num_iter)} / {str(max_iter)}")

pySPFM/decomposition.py

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,9 @@ class SparseDeconvolution(_BaseDeconvolution):
228228
tol : float, default=1e-6
229229
Convergence tolerance.
230230
n_jobs : int, default=1
231-
Number of parallel jobs. Only used in univariate mode (group=0).
231+
Number of parallel jobs for the per-voxel LARS criteria ('bic', 'aic').
232+
The FISTA criteria solve all voxels in a single batched call, so n_jobs
233+
has no effect on them.
232234
In multivariate mode, computation is inherently joint.
233235
positive : bool, default=False
234236
If True, enforce non-negative coefficients.
@@ -438,29 +440,35 @@ def _fit_lars(self, X, n_scans, n_voxels):
438440
self.lambda_[vox_idx] = np.squeeze(results[vox_idx][1])
439441

440442
def _fit_fista(self, X, n_scans, n_voxels):
441-
"""Fit using FISTA algorithm (univariate, voxel-wise)."""
442-
futures = []
443-
for vox_idx in range(n_voxels):
444-
fut = delayed_dask(fista, pure=False)(
445-
self.hrf_matrix_,
446-
X[:, vox_idx],
447-
criterion=self.criterion,
448-
max_iter=self.max_iter,
449-
min_iter=self.min_iter,
450-
tol=self.tol,
451-
group=0.0, # Univariate mode: no grouping
452-
pcg=self.pcg,
453-
factor=self.factor,
454-
lambda_echo=self.lambda_echo,
455-
positive_only=self.positive,
456-
)
457-
futures.append(fut)
458-
459-
results = self._dask_compute(futures)
443+
"""Fit using FISTA algorithm (univariate); all voxels in one batched call.
444+
445+
With ``group=0`` the proximal operator is element-wise, so a single
446+
``fista`` call over the full ``(n_scans, n_voxels)`` matrix is equivalent
447+
to solving each voxel independently -- but it replaces ``n_voxels``
448+
separate solves with one batched GEMM, which is ~50x faster on
449+
whole-brain data and avoids the per-voxel dispatch overhead. ``n_jobs``
450+
therefore has no effect here; it still applies to the LARS criteria.
451+
"""
452+
coef, lambda_ = fista(
453+
self.hrf_matrix_,
454+
X,
455+
criterion=self.criterion,
456+
max_iter=self.max_iter,
457+
min_iter=self.min_iter,
458+
tol=self.tol,
459+
group=0.0, # Univariate mode: element-wise (per-voxel) lasso prox
460+
pcg=self.pcg,
461+
factor=self.factor,
462+
lambda_echo=self.lambda_echo,
463+
positive_only=self.positive,
464+
)
460465

461-
for vox_idx in range(n_voxels):
462-
self.coef_[:, vox_idx] = np.squeeze(results[vox_idx][0])
463-
self.lambda_[vox_idx] = np.squeeze(results[vox_idx][1])
466+
self.coef_ = np.asarray(coef).reshape(n_scans, n_voxels)
467+
# select_lambda returns one lambda per voxel (or a scalar for 'eigval');
468+
# broadcast to the per-voxel vector the API promises.
469+
self.lambda_ = np.broadcast_to(
470+
np.asarray(lambda_, dtype=float).ravel(), (n_voxels,)
471+
).copy()
464472

465473
def _fit_fista_multivariate(self, X, n_scans, n_voxels):
466474
"""Fit using FISTA algorithm (multivariate, joint spatial regularization).

pySPFM/tests/test_fista.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,33 @@ def test_fista_weights_validation(sim_data, sim_hrf):
110110
# Weights are not supported with the pylops backend
111111
with pytest.raises(ValueError, match="pylops"):
112112
fista(hrf_matrix, y, group=0.2, max_iter=5, use_pylops=True, weights=np.ones(n_voxels))
113+
114+
115+
def test_fista_univariate_batched_equals_per_voxel(sim_data, sim_hrf):
116+
"""Univariate (group=0) FISTA over all voxels at once == solving each voxel alone.
117+
118+
SparseDeconvolution relies on this equivalence to batch the univariate path
119+
into a single solve instead of one call per voxel (~50x faster on whole-brain
120+
data). The lasso proximal operator is element-wise, so batching changes only
121+
the convergence gating, not the per-voxel result (up to float32 rounding).
122+
"""
123+
y = np.load(sim_data, allow_pickle=True)
124+
hrf_matrix = np.load(sim_hrf, allow_pickle=True)
125+
126+
batched, lambdas = fista(hrf_matrix, y, group=0.0, criterion="ut", max_iter=100)
127+
batched = np.asarray(batched)
128+
129+
# The batched solve covers all voxels; only loop over a small deterministic
130+
# subset for the per-voxel comparison so the test stays fast in CI.
131+
n_check = min(5, y.shape[1])
132+
per_voxel = np.zeros((batched.shape[0], n_check))
133+
for v in range(n_check):
134+
coef_v, _ = fista(hrf_matrix, y[:, v], group=0.0, criterion="ut", max_iter=100)
135+
per_voxel[:, v] = np.squeeze(np.asarray(coef_v))
136+
batched_subset = batched[:, :n_check]
137+
138+
# One lambda per voxel, identical sparse support, near-identical amplitudes.
139+
assert np.asarray(lambdas).reshape(-1).shape == (y.shape[1],)
140+
support_match = (np.abs(batched_subset) > 1e-6) == (np.abs(per_voxel) > 1e-6)
141+
assert support_match.mean() > 0.99
142+
assert np.allclose(batched_subset, per_voxel, atol=1e-3)

0 commit comments

Comments
 (0)