Skip to content

Commit 64db01c

Browse files
committed
Remove some duplicate calls to _norm() by adding an unsafe pdf evaluation method. Also fix an off-by-one error in the gamma evaluation that occurs when the evaluated gamma is on the pre-calculated grid.
1 parent 0e284ea commit 64db01c

3 files changed

Lines changed: 360 additions & 36 deletions

File tree

kingmaker/pdf.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,47 @@ def norm(
4949
"""
5050
return _norm(alpha, beta, self.angular_cutoff)
5151

52+
def pdf_from_norm(
53+
self,
54+
x: Union[float, npt.NDArray[np.floating]],
55+
alpha: Union[float, npt.NDArray[np.floating]],
56+
beta: Union[float, npt.NDArray[np.floating]],
57+
norm: Union[float, npt.NDArray[np.floating]],
58+
) -> Union[float, npt.NDArray[np.floating]]:
59+
"""
60+
Evaluate the King kernel given a precomputed normalization constant.
61+
62+
Equivalent to ``norm * unnormalized_pdf(x, alpha, beta)``, skipping
63+
the alpha/beta validation, angular-cutoff masking, and norm
64+
computation that :meth:`pdf` performs. Intended for callers that
65+
have already validated alpha/beta (e.g. against a previously-fit
66+
parameter grid) and already filtered ``x`` to be within
67+
``self.angular_cutoff``, and that can compute norm once over a
68+
smaller deduplicated (alpha, beta) grid than ``len(x)`` -- e.g.
69+
:meth:`kingmaker.wrapper.KingSpatialLikelihood.set_events`.
70+
71+
Parameters
72+
----------
73+
x : float or ndarray
74+
Angular separation(s) in radians. Must already be
75+
``<= self.angular_cutoff``; this is NOT checked.
76+
alpha : float or ndarray
77+
King distribution alpha parameter (scale) in radians. Must
78+
already be > 0; this is NOT checked.
79+
beta : float or ndarray
80+
King distribution beta parameter (tail weight). Must already
81+
be > 1; this is NOT checked.
82+
norm : float or ndarray
83+
Precomputed normalization constant(s), e.g. from :meth:`norm`.
84+
85+
Returns
86+
-------
87+
float or ndarray
88+
``norm * unnormalized_pdf(x, alpha, beta)``, broadcast over
89+
inputs.
90+
"""
91+
return norm * _unnormalized_pdf(x, alpha, beta)
92+
5293
def pdf(
5394
self,
5495
x: Union[float, npt.NDArray[np.floating]],

kingmaker/wrapper.py

Lines changed: 74 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,15 @@ def __init__(
133133

134134
# Instantiate the PDF object.
135135
self.king_pdf = KingPDF(angular_cutoff=angular_cutoff)
136+
137+
# Precompute the normalization constant for every (gamma, bin)
138+
# combination on the small fitted grid once per process. set_events
139+
# gathers per-event norm from this array via the same nearest-bin
140+
# indices used for alpha/beta, instead of recomputing the same
141+
# closed-form norm independently for every event that shares a bin
142+
# (most events in a trial do, since the grid is much smaller than
143+
# the event count).
144+
self.norm_values = self.king_pdf.norm(self.alpha_values, self.beta_values)
136145
return
137146

138147
def _events_match(self, events: npt.NDArray[Any]) -> bool:
@@ -248,14 +257,55 @@ def set_events(
248257
self.event_mask = all_dists < cutoff
249258
self.event_distances = all_dists[self.event_mask]
250259

251-
self._result_buffer = np.zeros(len(events))
260+
# Reuse the buffer across trials when the event count hasn't
261+
# changed, instead of reallocating every call. Either way, the
262+
# buffer is always fully zeroed here: the masked-out region is
263+
# write-only-once (only evaluate_pdf writes, and only at
264+
# [self.event_mask] positions), so this is the only place a stale
265+
# value from a previous trial's different mask could otherwise leak
266+
# through.
267+
if len(self._result_buffer) != len(events):
268+
self._result_buffer = np.zeros(len(events))
269+
else:
270+
self._result_buffer[:] = 0.0
252271

253-
all_alpha, all_beta = self.get_alpha_beta(events, copy=False)
272+
all_alpha, all_beta, all_norm = self._lookup_event_grid(events)
254273
for i, gamma in enumerate(self.spectral_indices):
255-
alpha, beta = self.get_alpha_beta_gamma(gamma, alpha=all_alpha, beta=all_beta)
256-
self.event_pvalue[gamma] = self.king_pdf.pdf(self.event_distances, alpha, beta)
274+
# alpha/beta are from the fitted grid (already validated at fit
275+
# time) and event_distances is already <= angular_cutoff by
276+
# construction above, so bypass KingPDF.pdf()'s validation/
277+
# masking and go straight to the kernel using the precomputed
278+
# per-bin norm. `i` is already the correct row (spectral_indices
279+
# and the leading axis of alpha_values/beta_values/norm_values
280+
# are index-aligned 1:1), so there's no need to go through
281+
# get_alpha_beta_gamma's searchsorted + defensive copy here.
282+
self.event_pvalue[gamma] = self.king_pdf.pdf_from_norm(
283+
self.event_distances, all_alpha[i], all_beta[i], all_norm[i]
284+
)
257285
return
258286

287+
def _lookup_event_grid(self, events):
288+
"""
289+
Nearest-bin lookup of alpha, beta, and norm for each (unmasked) event.
290+
291+
Shared by :meth:`get_alpha_beta` and :meth:`set_events` so the
292+
nearest-bin index computation is only ever done once per call.
293+
"""
294+
295+
# Nearest-bin lookup. Field-first masking (events[key][mask]) avoids
296+
# copying the full structured array before extracting each field.
297+
def index(centers, values):
298+
i = np.searchsorted(centers, values).clip(1, len(centers) - 1)
299+
return np.where(values - centers[i - 1] < centers[i] - values, i - 1, i)
300+
301+
event_indices = tuple(
302+
index(self.bin_centers[i], events[key][self.event_mask])
303+
for i, key in enumerate(self.keys)
304+
)
305+
306+
idx = (slice(None), *event_indices)
307+
return self.alpha_values[idx], self.beta_values[idx], self.norm_values[idx]
308+
259309
def get_alpha_beta(self, events, copy=True):
260310
"""
261311
Look up fitted alpha/beta parameters for each event via nearest-bin lookup.
@@ -282,20 +332,7 @@ def get_alpha_beta(self, events, copy=True):
282332
beta : ndarray, shape (n_gamma, n_masked_events)
283333
Fitted beta values for each spectral index and event.
284334
"""
285-
286-
# Nearest-bin lookup. Field-first masking (events[key][mask]) avoids
287-
# copying the full structured array before extracting each field.
288-
def index(centers, values):
289-
i = np.searchsorted(centers, values).clip(1, len(centers) - 1)
290-
return np.where(values - centers[i - 1] < centers[i] - values, i - 1, i)
291-
292-
event_indices = tuple(
293-
index(self.bin_centers[i], events[key][self.event_mask])
294-
for i, key in enumerate(self.keys)
295-
)
296-
297-
alpha = self.alpha_values[(slice(None), *event_indices)]
298-
beta = self.beta_values[(slice(None), *event_indices)]
335+
alpha, beta, _ = self._lookup_event_grid(events)
299336
return alpha, beta
300337

301338
def get_alpha_beta_gamma(self, gamma, events=None, alpha=None, beta=None):
@@ -336,28 +373,26 @@ def get_alpha_beta_gamma(self, gamma, events=None, alpha=None, beta=None):
336373
assert len(alpha) == len(beta)
337374

338375
# If we have this gamma, just return it. Make sure to use copy()
339-
# so the caller doesn't get a reference into our array.
376+
# so the caller doesn't get a reference into our array. Note:
377+
# searchsorted's default side="left" already returns the exact
378+
# index of an exact match in a sorted array -- no "-1" needed here
379+
# (that would be an off-by-one, wrapping to the previous row).
340380
if gamma in self.spectral_indices:
341-
gamma_idx = np.searchsorted(self.spectral_indices, gamma) - 1
381+
gamma_idx = np.searchsorted(self.spectral_indices, gamma)
342382
return alpha[gamma_idx].copy(), beta[gamma_idx].copy()
343383

344-
# Otherwise, we want to interpolate.
345-
gamma_low = max(np.searchsorted(self.spectral_indices, gamma) - 1, 0)
346-
gamma_high = min(gamma_low, len(self.spectral_indices - 1))
384+
# Otherwise, interpolate between the bracketing pair of spectral
385+
# indices, clamping so the pair is always valid even when gamma is
386+
# outside the stored range (extrapolation from the nearest pair).
387+
# Mirrors evaluate_pdf's bracketing logic.
388+
idx = np.clip(
389+
np.searchsorted(self.spectral_indices, gamma) - 1, 0, len(self.spectral_indices) - 2
390+
)
391+
gamma_low, gamma_high = self.spectral_indices[idx], self.spectral_indices[idx + 1]
347392

348393
return (
349-
_interp1d(
350-
self.spectral_indices[gamma_low],
351-
self.spectral_indices[gamma_high],
352-
alpha[gamma_low],
353-
alpha[gamma_high],
354-
),
355-
_interp1d(
356-
self.spectral_indices[gamma_low],
357-
self.spectral_indices[gamma_high],
358-
beta[gamma_low],
359-
beta[gamma_high],
360-
),
394+
_interp1d(gamma, gamma_low, gamma_high, alpha[idx], alpha[idx + 1]),
395+
_interp1d(gamma, gamma_low, gamma_high, beta[idx], beta[idx + 1]),
361396
)
362397

363398
def evaluate_pdf(self, events: npt.NDArray[Any], gamma: float = 2) -> npt.NDArray[np.floating]:
@@ -404,7 +439,10 @@ def evaluate_pdf(self, events: npt.NDArray[Any], gamma: float = 2) -> npt.NDArra
404439
)
405440

406441
gamma_low, gamma_high = self.spectral_indices[idx], self.spectral_indices[idx + 1]
407-
self._result_buffer[:] = 0.0
442+
# No need to re-zero the buffer here: event_mask is fixed for the
443+
# whole trial (only set_events changes it), and set_events already
444+
# zeroed the full buffer on entry, so the masked-out region is
445+
# already correct and untouched by anything else.
408446
self._result_buffer[self.event_mask] = _interp1d(
409447
gamma,
410448
gamma_low,

0 commit comments

Comments
 (0)