Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: Clarify exception message applies only to profile likelihood ratio #1806

Merged
merged 2 commits into from
Mar 8, 2022
Merged

refactor: Clarify exception message applies only to profile likelihood ratio #1806

merged 2 commits into from
Mar 8, 2022

Conversation

matthewfeickert
Copy link
Member

Description

Clarify that the warning RE: 'inference [being] impossible' the user gets if pyhf.infer.hypotest is called with fixed POIs is limited to profile likelihood ratio based methods, and not to methods like MLE fits. Thanks to a discussion with @alexander-held for helping me realize this warning wasn't very clear in general.

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
* Clarify that the message RE: 'inference [being] impossible' the user
gets if pyhf.infer.hypotest is called with fixed POIs is limited to
profile likelihood ratio based methods, and not to methods like MLE fits.

@matthewfeickert matthewfeickert added the refactor A code change that neither fixes a bug nor adds a feature label Mar 7, 2022
@matthewfeickert matthewfeickert self-assigned this Mar 7, 2022
alexander-held
alexander-held approved these changes Mar 8, 2022
View reviewed changes
Copy link
Member

@alexander-held alexander-held left a comment

Choose a reason for hiding this comment

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

Thanks for this addition, I think this is quite useful to avoid potential confusion. Looks good to me!

@matthewfeickert matthewfeickert merged commit 47ae6ad into master Mar 8, 2022
@matthewfeickert matthewfeickert deleted the fix/revise-warning branch March 8, 2022 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexander-held alexander-held alexander-held approved these changes

@kratsg kratsg kratsg approved these changes

@lukasheinrich lukasheinrich Awaiting requested review from lukasheinrich

Assignees

@matthewfeickert matthewfeickert

Labels
refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@matthewfeickert @kratsg @alexander-held