Skip to content

Commit 8f08909

Browse files
aarmeyclaude
andauthored
Speed up the test suite without dropping coverage (#1011)
* Speed up the test suite without dropping coverage The two slowest tests spent most of their time in StateDistribution.division_probability(), which numerically integrates a product of scipy.stats frozen-distribution pdf/sf calls; that generic rv_continuous machinery costs far more than the arithmetic itself when called ~200 times per quad() evaluation. Rewriting the integrand against scipy.special (gammaln/gammaincc) directly cuts test_works_through_crossval from ~45s to ~18s with identical results. Also enable pytest-xdist (-n auto, already a dev dependency but unused) and pin BLAS/OpenMP thread counts to 1 in a new tests/conftest.py so xdist's worker processes don't fight scipy's own internal threading for cores. Combined, the full suite drops from ~125s to ~47s. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix wrong Cython fused-function overload for digamma (psi) lineage/states/stateCommon.py grabbed the raw address of scipy.special.cython_special.__pyx_fuse_0psi and called it through a ctypes CFUNCTYPE(c_double, c_double). That symbol is psi's *complex128* specialization ("__pyx_t_double_complex (__pyx_t_double_complex, int)"); the real-valued one used here is __pyx_fuse_1psi. Calling the complex overload through a real signature reads garbage for the missing imaginary half of the argument. It happened to look right under plain ctypes calls (the unused slot came back zero by luck), which is why this went unnoticed, but numba's own generated call sequence for the same function pointer leaves that slot non-zero, so every njit call to psi() silently returned a wrong value. This directly biased two things: - gamma_mle_closed_form's Newton-Raphson solve for the shape parameter (its "100% exact, instantaneous" uncensored fast path), e.g. a synthetic Gamma(4, 2) sample recovered shape 3.84 instead of 3.98. - gamma_estimator's analytic gradient for uncensored observations, which was off by roughly 5-10x in the shape-parameter direction. That bad gradient is why SLSQP kept hitting its 200-iteration cap on otherwise well-posed censored fits and falling back to the much slower trust-constr solver, which was the dominant cost in test_cv[3] and test_works_through_crossval. Fixing the symbol lookup resolves both: the full test suite drops from ~47s to ~22s on top of the earlier speedups, and the shape MLE is now unbiased. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Vectorize StateDistributionCR.logpdf against scipy.special logpdf() runs once per state on every E step over the whole lineage array, but still went through the frozen rv_continuous machinery (self.div_clock.logpdf/logsf, self.death_clock.logpdf/logsf) for that - the same class of overhead already fixed in division_probability(), just not applied here. Replaced with direct vectorized formulas against scipy.special (gammaln/gammaincc), matching the old output to float precision (verified numerically) while cutting test_works_through_crossval from ~14.7s to ~6.3s in isolation. The div_clock/death_clock properties are left in place since compare_emissions.py still uses them outside this hot path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent aaaf66c commit 8f08909

4 files changed

Lines changed: 56 additions & 10 deletions

File tree

‎lineage/states/StateDistributionCR.py‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import scipy.stats as sp
3131
from scipy.integrate import quad
3232
from scipy.sparse import csr_array
33+
from scipy.special import gammaincc, gammaln
3334

3435
from .stateCommon import censor_lineage_gamma, censor_lineage_gaphs, gamma_estimator
3536

@@ -38,6 +39,16 @@
3839
TIME_FLOOR = 1e-10
3940

4041

42+
def _gamma_logpdf(t: np.ndarray, a: float, scale: float) -> np.ndarray:
43+
"""log Gamma(a, scale) pdf, vectorized directly against scipy.special."""
44+
return (a - 1.0) * np.log(t) - t / scale - a * np.log(scale) - gammaln(a)
45+
46+
47+
def _gamma_logsf(t: np.ndarray, a: float, scale: float) -> np.ndarray:
48+
"""log Gamma(a, scale) survival function, vectorized directly against scipy.special."""
49+
return np.log(gammaincc(a, t / scale))
50+
51+
4152
def event_masks(x: np.ndarray) -> tuple[np.ndarray, np.ndarray, np.ndarray]:
4253
"""Split cells into the three competing-risks cases.
4354
@@ -197,13 +208,23 @@ def death_clock(self):
197208
return sp.gamma(a=self.params[3], scale=self.params[4])
198209

199210
def division_probability(self) -> float:
200-
"""P(the division clock fires first) = int f_D(t) S_X(t) dt."""
201-
div, death = self.div_clock, self.death_clock
211+
"""P(the division clock fires first) = int f_D(t) S_X(t) dt.
212+
213+
``quad`` evaluates the integrand at ~100-200 points per call, and this runs
214+
once per state per M step. Using the frozen ``rv_continuous`` machinery for
215+
each point (as ``div.pdf`` / ``death.sf`` do) costs orders of magnitude more
216+
than the arithmetic itself, so the integrand is written directly against
217+
``scipy.special`` instead.
218+
"""
219+
a1, s1 = self.params[1], self.params[2]
220+
a2, s2 = self.params[3], self.params[4]
221+
log_norm = -gammaln(a1) - a1 * np.log(s1)
202222

203223
def integrand(t):
204-
return div.pdf(t) * death.sf(t)
224+
logpdf = log_norm + (a1 - 1.0) * np.log(t) - t / s1
225+
return np.exp(logpdf) * gammaincc(a2, t / s2)
205226

206-
upper = div.ppf(1.0 - 1e-9)
227+
upper = self.div_clock.ppf(1.0 - 1e-9)
207228
val = quad(integrand, 0.0, upper, limit=100)[0]
208229
return float(np.clip(val, 0.0, 1.0))
209230

@@ -231,19 +252,26 @@ def dof(self) -> int:
231252
return 3 if self.fixed_death_shape else 4
232253

233254
def logpdf(self, x: np.ndarray) -> np.ndarray:
234-
"""Competing-risks log likelihood of each cell's phase observation."""
255+
"""Competing-risks log likelihood of each cell's phase observation.
256+
257+
Called once per state on every E step, over the whole lineage array, so it is
258+
vectorized directly against ``scipy.special`` rather than going through the
259+
frozen ``rv_continuous`` machinery (``self.div_clock.logpdf`` etc.), which pays
260+
generic validation/broadcasting overhead on every call regardless of array size.
261+
"""
235262
divided, died, censored = event_masks(x)
236263
timed = divided | died | censored
237264
t = np.clip(x[:, 1], TIME_FLOOR, None)
238265

239-
div, death = self.div_clock, self.death_clock
266+
a1, s1 = self.params[1], self.params[2]
267+
a2, s2 = self.params[3], self.params[4]
240268

241269
ll = np.zeros(x.shape[0])
242270
# Every timed cell survived both clocks up to t; the one that fired then
243271
# swaps its survival term for a density.
244-
ll[timed] += div.logsf(t[timed]) + death.logsf(t[timed])
245-
ll[divided] += div.logpdf(t[divided]) - div.logsf(t[divided])
246-
ll[died] += death.logpdf(t[died]) - death.logsf(t[died])
272+
ll[timed] += _gamma_logsf(t[timed], a1, s1) + _gamma_logsf(t[timed], a2, s2)
273+
ll[divided] += _gamma_logpdf(t[divided], a1, s1) - _gamma_logsf(t[divided], a1, s1)
274+
ll[died] += _gamma_logpdf(t[died], a2, s2) - _gamma_logsf(t[died], a2, s2)
247275

248276
assert not np.any(np.isnan(ll))
249277
return ll

‎lineage/states/stateCommon.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,11 @@ def bern_estimator(bern_obs: np.ndarray, gammas: np.ndarray):
182182
_addr_gammaln = get_cython_function_address("scipy.special.cython_special", "gammaln")
183183
gammaln = CFUNCTYPE(c_double, c_double)(_addr_gammaln)
184184

185-
_addr_psi = get_cython_function_address("scipy.special.cython_special", "__pyx_fuse_0psi")
185+
# psi is a fused Cython function with both a complex and a real specialization;
186+
# __pyx_fuse_0psi is the complex128 one (silently reads garbage for the missing
187+
# imaginary half when called through this real-valued CFUNCTYPE) and __pyx_fuse_1psi
188+
# is the real double one we actually want.
189+
_addr_psi = get_cython_function_address("scipy.special.cython_special", "__pyx_fuse_1psi")
186190
psi = CFUNCTYPE(c_double, c_double)(_addr_psi)
187191

188192

‎lineage/tests/conftest.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
"""Test-session setup.
2+
3+
Several tests drive scipy/numpy linear algebra hard enough that BLAS's own thread
4+
pool competes with pytest-xdist's worker processes for cores, which slows the suite
5+
down rather than speeding it up. Pin each library to a single BLAS thread so the
6+
parallelism comes from xdist alone; this must happen before numpy/scipy are
7+
imported anywhere, so it lives at the top of the first conftest.py loaded.
8+
"""
9+
10+
import os
11+
12+
for _var in ("OMP_NUM_THREADS", "OPENBLAS_NUM_THREADS", "MKL_NUM_THREADS", "NUMEXPR_NUM_THREADS"):
13+
os.environ.setdefault(_var, "1")

‎pyproject.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,5 +73,6 @@ minversion = "6.0"
7373
testpaths = [
7474
"lineage/tests",
7575
]
76+
addopts = "-n auto"
7677

7778

0 commit comments

Comments
 (0)