From 3815e538c66410c73d847a381a9832bd6b815207 Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Fri, 9 Sep 2022 02:47:32 +0200 Subject: [PATCH 1/5] fix: Use '<=' over '<' to avoid nan in test stat calculation --- src/pyhf/infer/calculators.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pyhf/infer/calculators.py b/src/pyhf/infer/calculators.py index 8e06a613cb..ed9642e085 100644 --- a/src/pyhf/infer/calculators.py +++ b/src/pyhf/infer/calculators.py @@ -418,8 +418,9 @@ def _false_case(): teststat = (qmu - qmu_A) / (2 * self.sqrtqmuA_v) return teststat + # Use '<=' rather than '<' to avoid Issue #1992 teststat = tensorlib.conditional( - (sqrtqmu_v < self.sqrtqmuA_v), _true_case, _false_case + (sqrtqmu_v <= self.sqrtqmuA_v), _true_case, _false_case ) return tensorlib.astensor(teststat) From 52a5b41ddd215ec232e44a9c54de52a8d0659793 Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Fri, 9 Sep 2022 16:01:19 +0200 Subject: [PATCH 2/5] docs: Add inequality --- src/pyhf/infer/test_statistics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pyhf/infer/test_statistics.py b/src/pyhf/infer/test_statistics.py index e23e87fa75..3af773d42c 100644 --- a/src/pyhf/infer/test_statistics.py +++ b/src/pyhf/infer/test_statistics.py @@ -71,7 +71,7 @@ def qmu(mu, data, pdf, init_pars, par_bounds, fixed_params, return_fitted_pars=F \begin{equation} q_{\mu} = \left\{\begin{array}{ll} - -2\ln\lambda\left(\mu\right), &\hat{\mu} < \mu,\\ + -2\ln\lambda\left(\mu\right), &\hat{\mu} \leq \mu,\\ 0, & \hat{\mu} > \mu \end{array}\right. \end{equation} @@ -160,7 +160,7 @@ def qmu_tilde( \begin{equation} \tilde{q}_{\mu} = \left\{\begin{array}{ll} - -2\ln\tilde{\lambda}\left(\mu\right), &\hat{\mu} < \mu,\\ + -2\ln\tilde{\lambda}\left(\mu\right), &\hat{\mu} \leq \mu,\\ 0, & \hat{\mu} > \mu \end{array}\right. \end{equation} From 4145b4d5e64c1d0c3b404e6453bb611a739aef0a Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Fri, 9 Sep 2022 16:05:35 +0200 Subject: [PATCH 3/5] Use reverse condition '>' --- src/pyhf/infer/calculators.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/pyhf/infer/calculators.py b/src/pyhf/infer/calculators.py index ed9642e085..3906362fee 100644 --- a/src/pyhf/infer/calculators.py +++ b/src/pyhf/infer/calculators.py @@ -418,9 +418,11 @@ def _false_case(): teststat = (qmu - qmu_A) / (2 * self.sqrtqmuA_v) return teststat - # Use '<=' rather than '<' to avoid Issue #1992 + # Use '>' rather than reverse condition with '<=' to avoid + # equating floating point numbers. + # This comparison is done to avoid Issue #1992. teststat = tensorlib.conditional( - (sqrtqmu_v <= self.sqrtqmuA_v), _true_case, _false_case + (self.sqrtqmuA_v > sqrtqmu_v), _true_case, _false_case ) return tensorlib.astensor(teststat) From 8f9b799c2fc986b9cc37c82edb7dd0962feafebf Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Fri, 9 Sep 2022 16:15:44 +0200 Subject: [PATCH 4/5] Revert "Use reverse condition '>'" This reverts commit 4145b4d5e64c1d0c3b404e6453bb611a739aef0a. --- src/pyhf/infer/calculators.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/pyhf/infer/calculators.py b/src/pyhf/infer/calculators.py index 3906362fee..ed9642e085 100644 --- a/src/pyhf/infer/calculators.py +++ b/src/pyhf/infer/calculators.py @@ -418,11 +418,9 @@ def _false_case(): teststat = (qmu - qmu_A) / (2 * self.sqrtqmuA_v) return teststat - # Use '>' rather than reverse condition with '<=' to avoid - # equating floating point numbers. - # This comparison is done to avoid Issue #1992. + # Use '<=' rather than '<' to avoid Issue #1992 teststat = tensorlib.conditional( - (self.sqrtqmuA_v > sqrtqmu_v), _true_case, _false_case + (sqrtqmu_v <= self.sqrtqmuA_v), _true_case, _false_case ) return tensorlib.astensor(teststat) From 9d6e7f976f7551bec62487c4e6beadd4fa40f47a Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Fri, 9 Sep 2022 16:32:22 +0200 Subject: [PATCH 5/5] Add tests to guard against nan condition --- tests/test_infer.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_infer.py b/tests/test_infer.py index 92a3467585..8d06f57694 100644 --- a/tests/test_infer.py +++ b/tests/test_infer.py @@ -476,3 +476,37 @@ def test_fixed_poi(tmpdir, hypotest_args): pdf.config.param_set('mu').suggested_fixed = [True] with pytest.raises(pyhf.exceptions.InvalidModel): pyhf.infer.hypotest(*hypotest_args) + + +def test_teststat_nan_guard(): + # Example from Issue #1992 + model = pyhf.simplemodels.uncorrelated_background( + signal=[1.0], bkg=[1.0], bkg_uncertainty=[1.0] + ) + observations = [2] + test_poi = 0.0 + data = observations + model.config.auxdata + init_pars = model.config.suggested_init() + par_bounds = model.config.suggested_bounds() + fixed_params = model.config.suggested_fixed() + + test_stat = pyhf.infer.test_statistics.qmu_tilde( + test_poi, data, model, init_pars, par_bounds, fixed_params + ) + assert test_stat == pytest.approx(0.0) + asymptotic_calculator = pyhf.infer.calculators.AsymptoticCalculator( + data, model, test_stat="qtilde" + ) + # ensure not nan + assert ~np.isnan(asymptotic_calculator.teststatistic(test_poi)) + assert asymptotic_calculator.teststatistic(test_poi) == pytest.approx(0.0) + + # Example from Issue #529 + model = pyhf.simplemodels.uncorrelated_background([0.005], [28.0], [5.0]) + test_poi = 1.0 + data = [28.0] + model.config.auxdata + + test_results = pyhf.infer.hypotest( + test_poi, data, model, test_stat="qtilde", return_expected=True + ) + assert all(~np.isnan(result) for result in test_results)