Skip to content

fix: Correct out of bounds initialization parameter for impact notebook #1149

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.

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Oct 28, 2020

Description

Fix in docs/examples/notebooks/ImpactPlot.ipynb

E           ValueError                                Traceback (most recent call last)
E           <ipython-input-7-8b96e1f272ad> in <module>
E           ----> 1 impacts, labels = get_impact_data()
E           
E           <ipython-input-6-0d59a87fecbf> in get_impact_data()
E                33         if i % 5 == 0:
E                34             print(i)
E           ---> 35         impct = calc_impact(i, b[i], e[i], width, initv[i], model.config.poi_index)
E                36         impacts.append(impct - poi_free)
E                37     return np.asarray(impacts), labels
E           
E           <ipython-input-5-541db5dc867d> in calc_impact(idx, b, e, i, width, poi_index)
E                 6     poi_dn_post = bb[poi_index]
E                 7 
E           ----> 8     _, _, bb, ee = fitresults([(idx, b + width)])
E                 9     poi_up_pre = bb[poi_index]
E                10 
E           
E           <ipython-input-4-b266f6140696> in fitresults(constraints)
E                12         fixed_params[idx] = True
E                13 
E           ---> 14     result = pyhf.infer.mle.fit(
E                15         data, model, init_pars=init_pars, fixed_params=fixed_params, return_uncertainties=True
E                16     )
E           
E           ~/work/pyhf/pyhf/src/pyhf/infer/mle.py in fit(data, pdf, init_pars, par_bounds, fixed_params, **kwargs)
E               111     fixed_params = fixed_params or pdf.config.suggested_fixed()
E               112 
E           --> 113     _validate_fit_inputs(init_pars, par_bounds, fixed_params)
E               114 
E               115     # get fixed vals from the model
E           
E           ~/work/pyhf/pyhf/src/pyhf/infer/mle.py in _validate_fit_inputs(init_pars, par_bounds, fixed_params)
E                51     for par_idx, (value, bound) in enumerate(zip(init_pars, par_bounds)):
E                52         if not (bound[0] <= value <= bound[1]):
E           ---> 53             raise ValueError(
E                54                 f"fit initialization parameter (index: {par_idx}, value: {value}) lies outside of its bounds: {bound}"
E                55                 + "\nTo correct this adjust the initialization parameter values in the model spec or those given"
E           
E           ValueError: fit initialization parameter (index: 0, value: 1.9999586270208174) lies outside of its bounds: [0.915, 1.085]
E           To correct this adjust the initialization parameter values in the model spec or those given
E           as arguments to pyhf.infer.fit. If this value is intended, adjust the range of the parameter
E           bounds.

Checklist Before Requesting Reviewer

  • Tests are passing
  • Verified notebook workflow 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
* TBD
* Move 'non-asymptotic calculators' to Implemented list in README

@matthewfeickert matthewfeickert added docs Documentation related fix A bug fix labels Oct 28, 2020
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented Oct 28, 2020

Codecov Report

Merging #1149 (4c79e25) into master (d83771e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1149   +/-   ##
=======================================
  Coverage   97.43%   97.43%           
=======================================
  Files          63       63           
  Lines        3665     3665           
  Branches      522      522           
=======================================
  Hits         3571     3571           
  Misses         55       55           
  Partials       39       39           
Flag Coverage Δ
unittests 97.43% <ø> (ø)

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


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d83771e...4c79e25. Read the comment docs.

@matthewfeickert matthewfeickert force-pushed the fix/correct-fit-bounds-in-notebooks branch 2 times, most recently from 61d710f to 4a6c7e2 Compare October 29, 2020 02:41
@matthewfeickert matthewfeickert force-pushed the fix/correct-fit-bounds-in-notebooks branch from 4a6c7e2 to a22df05 Compare November 6, 2020 22:13
@matthewfeickert matthewfeickert changed the base branch from master to main September 21, 2022 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related fix A bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants