Skip to content

Commit c09efff

Browse files
committed
fix: cost function bugs (masked BinnedNLL gradient, pulls masks, visualize)
- BinnedNLL._grad: per-parameter renormalization correction for masked bins with >=2 parameters (was a scalar collapsing all parameters) - BinnedCost._n_err / LeastSquares._pulls: handle index masks correctly (~index_array is bitwise NOT) and OR the zero-error mask with the user mask instead of replacing it, so zero-error bins yield NaN pulls - CostSum.visualize: squeeze=False so a single visualizable component does not crash; drop the stale **kwargs docstring paragraph - UnbinnedNLL.scaled_pdf: scale by number of data points (data.shape[-1]) instead of D*N for multivariate data - _normalize_output: do not bypass shape validation for non-float outputs - BinnedNLL: cache total counts in _update_cache (avoid per-call np.sum); Template._pred: avoid per-component full-array temporaries for the variance floor - replace abc.abstractproperty with property over abstractmethod Assisted-by: ClaudeCode:claude-opus-4-8
1 parent 1b7478d commit c09efff

2 files changed

Lines changed: 142 additions & 19 deletions

File tree

src/iminuit/cost.py

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,16 @@ def _replace_none(x, replacement):
179179
return x
180180

181181

182+
def _exclusion_from_mask(mask: NDArray, n: int) -> NDArray:
183+
# Return a boolean array of length n that is True for excluded entries.
184+
# mask may be a boolean array or an array of indices of selected entries.
185+
if mask.dtype == bool:
186+
return ~mask
187+
excluded = np.ones(n, dtype=bool)
188+
excluded[mask] = False
189+
return excluded
190+
191+
182192
def chi2(y: ArrayLike, ye: ArrayLike, ym: ArrayLike) -> float:
183193
"""
184194
Compute (potentially) chi2-distributed cost.
@@ -758,16 +768,14 @@ def visualize(
758768
Dict that maps an index to dict of keyword arguments. This can be
759769
used to pass keyword arguments to a visualize method of a component with
760770
that index.
761-
**kwargs :
762-
Other keyword arguments are forwarded to all components.
763771
"""
764772
from matplotlib import pyplot as plt
765773

766774
n = sum(hasattr(comp, "visualize") for comp in self)
767775

768776
fig = plt.gcf()
769777
fig.set_figwidth(n * fig.get_figwidth() / 1.5)
770-
_, ax = plt.subplots(1, n, num=fig.number)
778+
_, ax = plt.subplots(1, n, num=fig.number, squeeze=False)
771779

772780
if component_kwargs is None:
773781
component_kwargs = {}
@@ -777,7 +785,7 @@ def visualize(
777785
if not hasattr(comp, "visualize"):
778786
continue
779787
kwargs = component_kwargs.get(k, {})
780-
plt.sca(ax[i])
788+
plt.sca(ax[0, i])
781789
comp.visualize(cargs, **kwargs)
782790
i += 1
783791

@@ -907,12 +915,14 @@ def __init__(
907915
self._model_grad = grad
908916
super().__init__(_model_parameters(model, name), _norm(data), verbose)
909917

910-
@abc.abstractproperty
918+
@property
919+
@abc.abstractmethod
911920
def pdf(self):
912921
"""Get probability density model."""
913922
... # pragma: no cover
914923

915-
@abc.abstractproperty
924+
@property
925+
@abc.abstractmethod
916926
def scaled_pdf(self):
917927
"""Get number density model."""
918928
... # pragma: no cover
@@ -1042,7 +1052,9 @@ def pdf(self):
10421052
@property
10431053
def scaled_pdf(self):
10441054
"""Get number density model."""
1045-
scale = np.prod(self.data.shape)
1055+
# number of data points: for multivariate data of shape (D, N) this
1056+
# is N, the last axis; for 1D data of shape (N,) it is also N
1057+
scale = self.data.shape[-1]
10461058
if self._log:
10471059
return lambda *args: scale * np.exp(self._model(*args))
10481060
return lambda *args: scale * self._model(*args)
@@ -1305,12 +1317,13 @@ class BinnedCost(MaskedCostWithPulls):
13051317
:meta private:
13061318
"""
13071319

1308-
__slots__ = "_xe", "_ndim", "_bohm_zech_n", "_bohm_zech_s"
1320+
__slots__ = "_xe", "_ndim", "_bohm_zech_n", "_bohm_zech_s", "_counts_total"
13091321

13101322
_xe: Union[NDArray, Tuple[NDArray, ...]]
13111323
_ndim: int
13121324
_bohm_zech_n: NDArray
13131325
_bohm_zech_s: Optional[NDArray]
1326+
_counts_total: float
13141327

13151328
n = MaskedCost.data
13161329

@@ -1434,7 +1447,11 @@ def _n_err(self) -> Tuple[NDArray, NDArray]:
14341447
# mask values where error is zero
14351448
ma = err == 0
14361449
if self.mask is not None:
1437-
ma = ~self.mask
1450+
# the mask acts on the first dimension; broadcast the per-bin
1451+
# exclusion across the remaining dimensions before combining
1452+
excluded = _exclusion_from_mask(self.mask, n.shape[0])
1453+
excluded = excluded.reshape((-1,) + (1,) * (n.ndim - 1))
1454+
ma = ma | excluded
14381455
n[ma] = np.nan
14391456
err[ma] = np.nan
14401457
return n, err
@@ -1460,6 +1477,8 @@ def _update_cache(self):
14601477
self._bohm_zech_n = val * s
14611478
else:
14621479
self._bohm_zech_n = n
1480+
# cache the total number of entries in the unmasked bins
1481+
self._counts_total = np.sum(self._counts())
14631482

14641483
def _transformed(self, val: NDArray) -> Tuple[NDArray, NDArray]:
14651484
s = self._bohm_zech_s
@@ -1793,8 +1812,8 @@ def __init__(
17931812
self._model_len = np.prod(self._xe_shape)
17941813

17951814
def _pred(self, args: Sequence[float]) -> Tuple[NDArray, NDArray]:
1796-
mu: NDArray = 0 # type:ignore
1797-
mu_var: NDArray = 0 # type:ignore
1815+
mu = np.zeros(self._data.shape[: self._ndim])
1816+
mu_var = np.zeros_like(mu)
17981817
i = 0
17991818
for t1, t2 in self._model_data:
18001819
if isinstance(t1, np.ndarray) and isinstance(t2, np.ndarray):
@@ -1813,7 +1832,9 @@ def _pred(self, args: Sequence[float]) -> Tuple[NDArray, NDArray]:
18131832
# subtraction, we set negative values to zero
18141833
d[d < 0] = 0
18151834
mu += d
1816-
mu_var += np.ones_like(mu) * 1e-300
1835+
# add a tiny floor to the variance to avoid exactly zero values;
1836+
# a scalar add avoids allocating full temporaries
1837+
mu_var += 1e-300
18171838
i += t2
18181839
else: # never arrive here
18191840
assert False # pragma: no cover
@@ -1986,7 +2007,7 @@ def _pred(self, args: Sequence[float]) -> NDArray:
19862007
if ma is not None:
19872008
p /= np.sum(p[ma])
19882009
# scale probabilities with total number of entries of unmasked bins in histogram
1989-
return p * np.sum(self._counts())
2010+
return p * self._counts_total
19902011

19912012
def _value(self, args: Sequence[float]) -> float:
19922013
mu = self._pred(args)
@@ -2001,11 +2022,11 @@ def _grad(self, args: Sequence[float]) -> NDArray:
20012022
# normalise probability of remaining bins
20022023
if ma is not None:
20032024
psum = np.sum(p[ma])
2004-
pg = pg / psum - p * np.sum(pg[:, ma]) / psum**2
2025+
pg = pg / psum - p * np.sum(pg[:, ma], axis=1)[:, np.newaxis] / psum**2
20052026
p /= psum
20062027
# scale probabilities with total number of entries of unmasked bins in histogram
20072028
n = self._counts()
2008-
ntot = np.sum(n)
2029+
ntot = self._counts_total
20092030
mu = p * ntot
20102031
gmu = pg * ntot
20112032
ma = self.mask
@@ -2345,10 +2366,11 @@ def _pulls(self, args: Sequence[float]) -> NDArray:
23452366
ye = self.yerror.copy()
23462367
ym = self.prediction(args)
23472368

2369+
ma = ye == 0
23482370
if self.mask is not None:
2349-
ma = ~self.mask
2350-
y[ma] = np.nan
2351-
ye[ma] = np.nan
2371+
ma = ma | _exclusion_from_mask(self.mask, y.shape[0])
2372+
y[ma] = np.nan
2373+
ye[ma] = np.nan
23522374
return (y - ym) / ye
23532375

23542376
def _pred(self, args: Sequence[float]) -> NDArray:
@@ -2551,7 +2573,7 @@ def _normalize_output(x, kind, *shape, msg=None):
25512573
warnings.warn(msg, PerformanceWarning)
25522574
x = np.array(x)
25532575
if x.dtype.kind != "f":
2554-
return x.astype(float)
2576+
x = x.astype(float)
25552577
if x.ndim < len(shape):
25562578
return x.reshape(*shape)
25572579
elif x.shape != shape:

tests/test_cost.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,21 @@ def test_UnbinnedNLL_properties(log):
384384
assert c.verbose == 1
385385

386386

387+
def test_UnbinnedNLL_scaled_pdf_2D():
388+
# for multivariate data of shape (D, N) the scale must be the number of
389+
# data points N, not D * N
390+
def model(x_y, mux, muy, sx, sy):
391+
return mvnorm(mux, muy, sx, sy).pdf(x_y.T)
392+
393+
truth = 0.1, 0.2, 0.3, 0.4
394+
x, y = mvnorm(*truth).rvs(size=15, random_state=1).T
395+
c = UnbinnedNLL((x, y), model)
396+
397+
assert c.data.shape == (2, 15)
398+
expected = 15 * model(c.data, *truth)
399+
assert_allclose(c.scaled_pdf(c.data, *truth), expected)
400+
401+
387402
@pytest.mark.parametrize("log", (False, True))
388403
def test_UnbinnedNLL_visualize(log):
389404
pytest.importorskip("matplotlib")
@@ -643,6 +658,30 @@ def test_BinnedNLL_pulls(binned):
643658
assert np.nanvar(pulls) == pytest.approx(1, abs=0.2)
644659

645660

661+
def test_BinnedNLL_pulls_mask():
662+
# masked bins must produce NaN pulls for both boolean and index masks
663+
n = np.array([5, 1000, 50, 1])
664+
xe = [0, 1, 2, 3, 4]
665+
c = BinnedNLL(n, xe, expon_cdf)
666+
args = (1.0,)
667+
668+
full = c.pulls(args)
669+
assert not np.any(np.isnan(full))
670+
671+
# boolean mask: masked bin is NaN, the rest finite
672+
c.mask = np.array([True, True, False, True])
673+
p = c.pulls(args)
674+
assert np.isnan(p[2])
675+
assert not np.any(np.isnan(p[[0, 1, 3]]))
676+
677+
# index mask selecting the same bins must give identical pulls;
678+
# bitwise NOT of an index array would poison the wrong bins
679+
c.mask = np.array([0, 1, 3])
680+
p2 = c.pulls(args)
681+
assert np.isnan(p2[2])
682+
assert_allclose(p2[[0, 1, 3]], p[[0, 1, 3]])
683+
684+
646685
@pytest.mark.parametrize("use_grad", (False, True))
647686
def test_BinnedNLL_weighted(use_grad):
648687
xe = np.array([0, 0.2, 0.4, 0.8, 1.5, 10])
@@ -840,6 +879,23 @@ def test_BinnedNLL_mask():
840879
assert_allclose(c.grad(2), ref(2))
841880

842881

882+
def test_BinnedNLL_mask_grad_multipar():
883+
# regression test: the masked gradient renormalization correction must be
884+
# per-parameter; with >=2 parameters a scalar correction is wrong
885+
pytest.importorskip("jacobi")
886+
xe = np.linspace(-2, 2, 6)
887+
n = np.diff(norm_cdf(xe, 0.1, 1.2)) * 1000
888+
c = BinnedNLL(n, xe, norm_cdf, grad=numerical_model_gradient(norm_cdf))
889+
c.mask = np.arange(len(n)) != 2
890+
891+
ref = numerical_cost_gradient(c)
892+
# evaluate away from the truth so the gradient is clearly non-zero
893+
for args in [(0.0, 1.0), (0.3, 0.8), (-0.2, 1.5)]:
894+
g = c.grad(*args)
895+
assert np.linalg.norm(g) > 1.0
896+
assert_allclose(g, ref(*args), rtol=1e-3)
897+
898+
843899
def test_BinnedNLL_properties():
844900
def cdf(x, a, b):
845901
return 0
@@ -1457,6 +1513,25 @@ def test_LeastSquares_pulls():
14571513
assert_equal(c.pulls((0, 1)), [10, np.nan])
14581514

14591515

1516+
def test_LeastSquares_pulls_mask_index():
1517+
# an index mask must mask the same bins as the equivalent boolean mask;
1518+
# bitwise NOT of an index array would poison the wrong bins
1519+
c = LeastSquares([1, 2, 3], [2, 3, 4], 0.1, line)
1520+
c.mask = [0, 2]
1521+
assert_equal(c.pulls((0, 1)), [10, np.nan, 10])
1522+
1523+
1524+
def test_LeastSquares_pulls_zero_error():
1525+
# bins with zero error must yield NaN pulls (documented), not +-inf,
1526+
# also when inside the user mask
1527+
c = LeastSquares([1, 2, 3], [2, 3, 4], [0.1, 0.0, 0.1], line)
1528+
p = c.pulls((0, 1))
1529+
assert_equal(p, [10, np.nan, 10])
1530+
c.mask = [True, True, True]
1531+
p = c.pulls((0, 1))
1532+
assert_equal(p, [10, np.nan, 10])
1533+
1534+
14601535
@pytest.mark.parametrize("use_grad", (False, True))
14611536
def test_CostSum_1(use_grad):
14621537
def model1(x, a):
@@ -1632,6 +1707,18 @@ def test_CostSum_visualize():
16321707
c.visualize((1, 2))
16331708

16341709

1710+
def test_CostSum_visualize_single_component():
1711+
# regression test: a CostSum with exactly one visualizable component must
1712+
# not crash; subplots(1, 1) returns a bare Axes unless squeeze=False
1713+
pytest.importorskip("matplotlib")
1714+
from matplotlib import pyplot as plt
1715+
1716+
c = UnbinnedNLL([1.0, 2.0, 3.0], norm_pdf) + 10.0
1717+
plt.figure()
1718+
c.visualize((0.0, 1.0))
1719+
plt.close("all")
1720+
1721+
16351722
def test_NormalConstraint_1():
16361723
c1 = NormalConstraint("a", 1, 1.5)
16371724
c2 = NormalConstraint(("a", "b"), (1, 2), (3, 4))
@@ -1791,6 +1878,20 @@ def test_NormalConstraint_pickle():
17911878
assert_equal(c.covariance, c2.covariance)
17921879

17931880

1881+
def test_normalize_output_wrong_shape_int():
1882+
# an integer array with the wrong shape must still raise the descriptive
1883+
# shape error instead of silently slipping through the float early-return
1884+
from iminuit.cost import _normalize_output
1885+
1886+
x = np.array([1, 2], dtype=int)
1887+
with pytest.warns(PerformanceWarning):
1888+
with pytest.raises(
1889+
ValueError,
1890+
match=r"output of model has shape \(2,\), but \(3,\) is required",
1891+
):
1892+
_normalize_output(list(x), "model", 3)
1893+
1894+
17941895
def test_NormalConstraint_bad_input_1():
17951896
with pytest.raises(ValueError, match="scalar or one-dimensional"):
17961897
NormalConstraint("par", [[[1, 2]]], np.eye(2))

0 commit comments

Comments
 (0)