Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ Changelog
IN PROGRESS
--------------------

* Add frequentist coverage interval support in the ``intervals`` module.
`#176 <https://github.com/scikit-hep/hist/pull/176>`_

* Allow ``plot_pull`` to take a more generic callable or a string as a fitting function.
Introduce an option to perform a likelihood fit. Write fit parameters' values
and uncertainties in the legend.
Expand Down
8 changes: 8 additions & 0 deletions docs/reference/hist.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ hist.hist module
:undoc-members:
:show-inheritance:

hist.intervals module
---------------------

.. automodule:: hist.intervals
:members:
:undoc-members:
:show-inheritance:

hist.namedhist module
---------------------

Expand Down
1 change: 0 additions & 1 deletion src/hist/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from .namedhist import NamedHist
from .tag import loc, overflow, rebin, sum, underflow

# Convenient access to the version number
# Convenient access to the version number
from .version import version as __version__

Expand Down
145 changes: 145 additions & 0 deletions src/hist/intervals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
from typing import Any, Optional, Tuple

import numpy as np

from .typing import Literal

try:
from scipy import stats
except ModuleNotFoundError:
from sys import stderr

print(
"hist.intervals requires scipy. Please install hist[plot] or manually install scipy.",
file=stderr,
)
raise

__all__ = ("poisson_interval", "clopper_pearson_interval", "ratio_uncertainty")


def __dir__() -> Tuple[str, ...]:
return __all__


def poisson_interval(
values: np.ndarray, variances: np.ndarray, coverage: "Optional[float]" = None
) -> np.ndarray:
r"""
The Frequentist coverage interval for Poisson-distributed observations.

What is calculated is the "Garwood" interval,
c.f. https://www.ine.pt/revstat/pdf/rs120203.pdf or
http://ms.mcmaster.ca/peter/s743/poissonalpha.html.
For weighted data, this approximates the observed count by
``values**2/variances``, which effectively scales the unweighted
Poisson interval by the average weight.
This may not be the optimal solution: see https://arxiv.org/abs/1309.1287
for a proper treatment.

When a bin is zero, the scale of the nearest nonzero bin is substituted to
scale the nominal upper bound.

Args:
values: Sum of weights.
variances: Sum of weights squared.
coverage: Central coverage interval.
Default is one standard deviation, which is roughly ``0.68``.

Returns:
The Poisson central coverage interval.
"""
# Parts originally contributed to coffea
# https://github.com/CoffeaTeam/coffea/blob/8c58807e199a7694bf15e3803dbaf706d34bbfa0/LICENSE
if coverage is None:
coverage = stats.norm.cdf(1) - stats.norm.cdf(-1)
scale = np.empty_like(values)
scale[values != 0] = variances[values != 0] / values[values != 0]
if np.sum(values == 0) > 0:
missing = np.where(values == 0)
available = np.nonzero(values)
if len(available[0]) == 0:
raise RuntimeError(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent with docstring. I would suggest warnings.warn with a RuntimeWarning.

Copy link
Member Author

@matthewfeickert matthewfeickert Apr 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Henry had suggested the RuntimeError and I had forgot to upate the docstring. Why should this be a warning though vs. an error? If everything is zero then something is wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's valid to plot an empty histogram with error bars, certainly if the data is not scaled then you even have a well-defined error bar. The interpretation of them can be challenging if the data is scaled, however, and there's no way to tell other than the user. Either way it seems we should not throw an exception.

Copy link
Member Author

@matthewfeickert matthewfeickert Apr 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's valid to plot an empty histogram with error bars

Why are you plotting non-existent data? If the histogram is actually empty this doesn't make sense. Link to an example?

Also, why are you taking a ratio of anything that is non-existent? This makes even less sense to me. :? An example would be helpful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this poisson interval method used for plain errorbar histos (as well as ratios)? For ratios sure makes less sense. But imagine you are dumping 100 region plots to a file, now if one region is empty do you want your plot dumper script to crash or emit a warning?

Copy link
Member Author

@matthewfeickert matthewfeickert Apr 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But imagine you are dumping 100 region plots to a file, now if one region is empty do you want your plot dumper script to crash or emit a warning?

IMO it should crash. It is your responsibility to clean your data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, its a valid opinion. Just seems at odds with the amount of work this routine does to make up vaguely reasonable error bars in the case where even all but one bin is zero, that it would give up if all are zero. I won't press the issue further then (except to update the docstring)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you expect it to crash, that's what try/except (or using "if" beforehand) is for. This could mask real problems, like all plots being empty because something is misconfigured? Nobody reads logs; warnings are next to useless.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this poisson interval method used for plain errorbar histos (as well as ratios)?

In practice it could be, but at the moment it is only being used in ratio_uncertainty.

Just seems at odds with the amount of work this routine does to make up vaguely reasonable error bars in the case where even all but one bin is zero, that it would give up if all are zero

I see what you're saying, but then would you also suggest not allowing any empty values? Or how would you define the cutoff? I don't think that you're pressing anything, I just want to make sure that I understand what your thoughts are here as it is clear that you've thought about this far more than I have.

(Henry I think we're in agreement but if I'm misunderstanding (sorry) please let me know).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The all zero case is the only situation when there really isn't enough info to do something reasonable so its fine. LGTM

"All values are zero! Cannot compute meaningful uncertainties.",
)
nearest = np.sum(
[np.square(np.subtract.outer(d, d0)) for d, d0 in zip(available, missing)]
).argmin(axis=0)
argnearest = tuple(dim[nearest] for dim in available)
scale[missing] = scale[argnearest]
counts = values / scale
interval_min = scale * stats.chi2.ppf((1 - coverage) / 2, 2 * counts) / 2.0
interval_max = scale * stats.chi2.ppf((1 + coverage) / 2, 2 * (counts + 1)) / 2.0
interval = np.stack((interval_min, interval_max))
interval[interval == np.nan] = 0.0 # chi2.ppf produces nan for counts=0
return interval


def clopper_pearson_interval(
num: np.ndarray, denom: np.ndarray, coverage: "Optional[float]" = None
) -> np.ndarray:
r"""
Compute the Clopper-Pearson coverage interval for a binomial distribution.
c.f. http://en.wikipedia.org/wiki/Binomial_proportion_confidence_interval

Args:
num: Numerator or number of successes.
denom: Denominator or number of trials.
coverage: Central coverage interval.
Default is one standard deviation, which is roughly ``0.68``.

Returns:
The Clopper-Pearson central coverage interval.
"""
# Parts originally contributed to coffea
# https://github.com/CoffeaTeam/coffea/blob/8c58807e199a7694bf15e3803dbaf706d34bbfa0/LICENSE
if coverage is None:
coverage = stats.norm.cdf(1) - stats.norm.cdf(-1)
# Numerator is subset of denominator
if np.any(num > denom):
raise ValueError(
"Found numerator larger than denominator while calculating binomial uncertainty"
)
interval_min = stats.beta.ppf((1 - coverage) / 2, num, denom - num + 1)
interval_max = stats.beta.ppf((1 + coverage) / 2, num + 1, denom - num)
interval = np.stack((interval_min, interval_max))
interval[:, num == 0.0] = 0.0
interval[1, num == denom] = 1.0
return interval


def ratio_uncertainty(
num: np.ndarray,
denom: np.ndarray,
uncertainty_type: Literal["poisson", "poisson-ratio"] = "poisson",
) -> Any:
r"""
Calculate the uncertainties for the values of the ratio ``num/denom`` using
the specified coverage interval approach.

Args:
num: Numerator or number of successes.
denom: Denominator or number of trials.
uncertainty_type: Coverage interval type to use in the calculation of
the uncertainties.
``"poisson"`` (default) implements the Poisson interval for the
numerator scaled by the denominator.
``"poisson-ratio"`` implements the Clopper-Pearson interval for Poisson
distributed ``num`` and ``denom``.

Returns:
The uncertainties for the ratio.
"""
# Note: As return is a numpy ufuncs the type is "Any"
with np.errstate(divide="ignore"):
ratio = num / denom
if uncertainty_type == "poisson":
ratio_uncert = np.abs(poisson_interval(ratio, num / np.square(denom)) - ratio)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe some more docs on what this is would be helpful? I am actually not sure what it is, poisson for numerator?

Copy link
Member

@nsmith- nsmith- Apr 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked back at my code to see that indeed it is poisson for numerator. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm switching work at the moment, but I'll come back tonight and clean this up and make it more clear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Poke me when ready. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Poke me when ready

sorry I missed this comment. poke.

elif uncertainty_type == "poisson-ratio":
# poisson ratio n/m is equivalent to binomial n/(n+m)
ratio_uncert = np.abs(clopper_pearson_interval(num, num + denom) - ratio)
else:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to add as well the simple propagation of error style uncertainty: https://github.com/CoffeaTeam/coffea/blob/84e805773c1fac32fc79bc9373ec324552371244/coffea/hist/plot.py#L82

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but is that needed in this PR to get things up and going for PR #161? That could be added later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to open as an issue and move forward with this for now. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering its the ROOT default ratio plot error (TH1::Divide) seems we would want that no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering its the ROOT default ratio plot error (TH1::Divide) seems we would want that no?

Let me go look, but I'm not sold on this argument. ROOT does plenty of things that are not a good idea.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also do not consider ROOT's warning vs. error behavior to be a design standard for us. :)

Now if there's a valid reason to do this, then why is there a warning? And if not, why not just error now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(If it's valid but rare, warnings can be squelched, while error's can't be)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't add it originally either, but it is valid in some cases. See the PR that added it to coffea: scikit-hep/coffea#182

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't add it originally either, but it is valid in some cases.

Looking back at that Issue it is brought up that

Ah yeah, fair enough, this would be needed for efficiencies derived from weighted data (which is kind of rare, but definitely happens).

But the specific example isn't really elaborated on. Can you either ELI5 why this is worth doing now, or make a new Issue from this discussion and it can get labeled as a "good first issue"? Again, you've thought about this all more than I have, so it is very possible I'm just not seeing something incredibly obvious to you.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it would be convenient to copy it as the others, since it isn't much work. But also fine to put it off to later if you prefer. I can make the issue

raise TypeError(
f"'{uncertainty_type}' is an invalid option for uncertainty_type."
)
return ratio_uncert
2 changes: 1 addition & 1 deletion src/hist/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ def plot_pull(
try:
from iminuit import Minuit # noqa: F401
from scipy.optimize import curve_fit # noqa: F401
except ImportError:
except ModuleNotFoundError:
print(
"Hist.plot_pull requires scipy and iminuit. Please install hist[plot] or manually install dependencies.",
file=sys.stderr,
Expand Down
6 changes: 3 additions & 3 deletions src/hist/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
from typing import TYPE_CHECKING, Any, Tuple

if sys.version_info < (3, 8):
from typing_extensions import Protocol, SupportsIndex
from typing_extensions import Literal, Protocol, SupportsIndex
else:
from typing import Protocol, SupportsIndex
from typing import Literal, Protocol, SupportsIndex

if TYPE_CHECKING:
from numpy import ufunc as Ufunc
Expand All @@ -14,7 +14,7 @@
Ufunc = Any


__all__ = ("Protocol", "SupportsIndex", "Ufunc", "ArrayLike")
__all__ = ("Literal", "Protocol", "SupportsIndex", "Ufunc", "ArrayLike")


def __dir__() -> Tuple[str, ...]:
Expand Down
141 changes: 141 additions & 0 deletions tests/test_intervals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import numpy as np
import pytest
from pytest import approx

from hist import Hist, axis, intervals


@pytest.fixture(scope="session")
def hist_fixture():
np.random.seed(42)

hist_1 = Hist(
axis.Regular(
10, -3, 3, name="X", label="x [units]", underflow=False, overflow=False
)
).fill(np.random.normal(size=1000))
hist_2 = Hist(
axis.Regular(
10, -3, 3, name="X", label="x [units]", underflow=False, overflow=False
)
).fill(np.random.normal(size=1700))

return hist_1, hist_2


def test_poisson_interval(hist_fixture):
hist_1, hist_2 = hist_fixture

interval_min, interval_max = intervals.poisson_interval(
hist_1.values(), hist_2.values()
)

assert approx(interval_min) == np.array(
[
1.5300291697782136,
15.163319280895097,
70.76628228209115,
138.53885398885032,
206.76205061547802,
216.04895966121967,
146.23699499970348,
59.34874385941129,
23.42140089960769,
4.398298999080774,
]
)
assert approx(interval_max) == np.array(
[
12.599700199673102,
28.738493673101413,
94.88918823365604,
173.30954997605485,
246.94963052163627,
257.713403993323,
181.58237748187338,
84.74029590412256,
38.20780361508876,
13.75903516119368,
]
)


def test_clopper_pearson_interval(hist_fixture):
hist_1, _ = hist_fixture

interval_min, interval_max = intervals.clopper_pearson_interval(
hist_1.values() * 0.8, hist_1.values()
)

assert approx(interval_min) == np.array(
[
0.4757493739959414,
0.6739341864914268,
0.745848569184471,
0.7626112365367469,
0.769793269861182,
0.7705165046817444,
0.7636693080808218,
0.7409965720106119,
0.6996942106437167,
0.5617068220945686,
]
)

assert approx(interval_max) == np.array(
[
0.9660393063397832,
0.8909499770371633,
0.8458913477820733,
0.8331466664953922,
0.8273390876155183,
0.8267415410793807,
0.832305086850061,
0.8493900886167576,
0.8763181169809199,
0.9404385982116935,
]
)


def test_ratio_uncertainty(hist_fixture):
hist_1, hist_2 = hist_fixture

uncertainty_min, uncertainty_max = intervals.ratio_uncertainty(
hist_1.values(), hist_2.values(), uncertainty_type="poisson"
)

assert approx(uncertainty_min) == np.array(
[
0.1439794096271186,
0.12988019998066708,
0.0711565635066328,
0.045722288708959336,
0.04049103990124614,
0.038474711321686006,
0.045227104349518155,
0.06135954973309016,
0.12378460125991042,
0.19774186117590858,
]
)

assert approx(uncertainty_max) == np.array(
[
0.22549817680979262,
0.1615766277480729,
0.07946632561746425,
0.04954668134626106,
0.04327624938437291,
0.04106267733757407,
0.04891233040201837,
0.06909296140898324,
0.1485919630151803,
0.2817958228477908,
]
)

with pytest.raises(TypeError):
intervals.ratio_uncertainty(
hist_1.values(), hist_2.values(), uncertainty_type="fail"
)