Skip to content

Conversation

@lorenzennio
Copy link
Contributor

@lorenzennio lorenzennio commented Dec 7, 2023

Description

When running

model = pyhf.simplemodels.uncorrelated_background(signal=[5.0, 10.0], bkg=[50.0, 60.0], bkg_uncertainty=[5.0, 12.0])
pyhf.infer.hypotest(1.0, [53.0, 65.0] + model.config.auxdata, model, test_stat="qmu")

we get a warning

qmu test statistic used for fit configuration with POI bounded at zero.
Use the qmu_tilde test statistic (pyhf.infer.test_statistics.qmu_tilde) instead.

and vice versa for POIs not bounded at 0.

But this should also tell you to set test_stat="qtilde" or (test_stat="q").

Checklist Before Requesting Reviewer

  • Tests are passing
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
* Add information to the user in the warning that provides them with the
  higher level pyhf.infer APIs kwarg to set the correct test statistic.

@codecov
Copy link

codecov bot commented Dec 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4d215a7) 96.29% compared to head (efe4ba2) 98.28%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2390      +/-   ##
==========================================
+ Coverage   96.29%   98.28%   +1.98%     
==========================================
  Files          69       69              
  Lines        4539     4539              
  Branches      648      803     +155     
==========================================
+ Hits         4371     4461      +90     
+ Misses        128       45      -83     
+ Partials       40       33       -7     
Flag Coverage Δ
contrib 97.86% <ø> (?)
doctest 60.71% <ø> (?)
unittests-3.10 96.29% <ø> (ø)
unittests-3.11 96.29% <ø> (?)
unittests-3.8 96.32% <ø> (?)
unittests-3.9 96.34% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matthewfeickert matthewfeickert added the need-to-backport tmp label until can be backported to patch release branch label Dec 8, 2023
Copy link
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

Thanks @lorenzennio. So this is good, but these log warnings are raised in the test stat APIs themselves, where there is no test_stat kwarg, as the API is already the test stat. So I would suggest on a newline (as this will get long otherwsise) noting that if the user is calling from the pyhf.infer.mle APIs or pyhf.infer.hypotest this can be set via kwarg.

@matthewfeickert matthewfeickert added the feat/enhancement New feature or request label Dec 8, 2023
@matthewfeickert matthewfeickert merged commit 6cd2de4 into scikit-hep:main Dec 8, 2023
matthewfeickert added a commit that referenced this pull request Jan 5, 2024
…I bounds (#2426)

* Backport PR #2390
* Add Lorenz Gaertner to contributors list.

Co-authored-by: Lorenz Gaertner <lorenz.gaertner@physik.uni-muenchen.de>
@matthewfeickert matthewfeickert removed the need-to-backport tmp label until can be backported to patch release branch label Jan 5, 2024
@matthewfeickert
Copy link
Member

@lorenzennio This fix is now out in pyhf v0.7.6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat/enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants