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.

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

Expand test for parameter uncertainties #1193

Closed
alexander-held opened this issue Nov 24, 2020 · 2 comments · Fixed by #1197
Closed

Expand test for parameter uncertainties #1193

alexander-held opened this issue Nov 24, 2020 · 2 comments · Fixed by #1197
Assignees
Labels
tests pytest

Comments

@alexander-held
Copy link
Member

Question

I was looking through #1183 and noticed that there does not seem to be a test to check the values of parameter uncertainties returned by minuit, which I would expect to be done here:

pyhf/tests/test_optim.py

Lines 384 to 408 in a33badd

@pytest.mark.parametrize('mu', [1.0], ids=['mu=1'])
@pytest.mark.only_numpy_minuit
def test_optim_uncerts(backend, source, spec, mu):
pdf = pyhf.Model(spec)
data = source['bindata']['data'] + pdf.config.auxdata
init_pars = pdf.config.suggested_init()
par_bounds = pdf.config.suggested_bounds()
optim = pyhf.optimizer
result = optim.minimize(pyhf.infer.mle.twice_nll, data, pdf, init_pars, par_bounds)
assert pyhf.tensorlib.tolist(result)
result = optim.minimize(
pyhf.infer.mle.twice_nll,
data,
pdf,
init_pars,
par_bounds,
[(pdf.config.poi_index, mu)],
return_uncertainties=True,
)
assert result.shape[1] == 2
assert pyhf.tensorlib.tolist(result)

I am happy to prepare a PR to add such a test, but wanted to check first whether this is of interest. I noticed it since that PR adds a HESSE call, which should slightly change the reported uncertainties, but did not find a corresponding change in the test.

Relevant Issues and Pull Requests

#1183

@lukasheinrich
Copy link
Contributor

I think more test coverage is always better. this way we can keep track when things change

@matthewfeickert matthewfeickert added the tests pytest label Nov 24, 2020
@matthewfeickert
Copy link
Member

@alexander-held Yeah I'll second that. A PR is very welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests pytest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants