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

fix: compatibility with pyhf 0.6.3 (parameter label API, auxdata) #248

Merged
merged 14 commits into from
Sep 7, 2021

Conversation

alexander-held
Copy link
Member

@alexander-held alexander-held commented Aug 6, 2021

scikit-hep/pyhf#1536 (see also discussion in scikit-hep/pyhf#867) added model.config.par_names() to the pyhf model config API. This provides the same functionality as cabinetry.model_utils.parameter_names, but the parameter names are now also passed through to MINUIT. A fix is included to pass the correct names when using MINOS (they used to be x0, x1 etc.). cabinetry.model_utils.parameter_names is removed, since the functionality is now available directly in pyhf.

The parameter naming format has changed: parameter[bin_0] -> parameter[0] etc., and the index is now included even for parameters like shapesys/shapefactors that only have a single component (because there is only a single bin in the region, see scikit-hep/pyhf#1560).

scikit-hep/pyhf#1562 introduces an API change for the auxdata kwarg naming, from with_aux to include_auxdata. cabinetry used with_aux also in its own API, and this now switches to include_auxdata for consistency with pyhf.

Due to a typing-extensions requirements conflict that results in the tensorflow-probability installation being incompatible with the installed tensorflow version (see also scikit-hep/pyhf#1595, tensorflow/tensorflow#51743), the pyhf[backends] installation is removed from the cabinetry test setup extra. It is now available via a new pyhf_backends setup extra and installed only when needed for the backend tests. That allows installing a working tensorflow setup (and picks up a conflict with black that does not matter at that point in the CI, as black already ran earlier).

Breaking changes:

  • removed cabinetry.model_utils.parameter_names in favor of the pyhf implementation model.config.par_names()
  • model_utils.model_and_data and model_utils.asimov_data: renamed with_aux kwarg to include_auxdata for consistency with pyhf
  • test setup extra no longer includes all pyhf backends, now moved to pyhf_backends setup extra
* breaking change: removed model_utils.parameter_names in favor of model.config.par_names provided by pyhf
* breaking change: renamed with_aux arguments in model_utils to include_auxdata for consistency with pyhf
* breaking change: pyhf backends removed from cabinetry test setup extra
* parameter naming format changed from parameter[bin_0] to parameter[0]
* raised minimum required pyhf version to v0.6.3

@matthewfeickert
Copy link
Member

scikit-hep/pyhf#1560 is probably going to affect this PR, given that this PR is in response to scikit-hep/pyhf#1536, but I haven't looked over it so you might be fine.

@alexander-held alexander-held force-pushed the fix/pyhf-parameter-names branch from 3898ed0 to c00332a Compare August 25, 2021 19:24
@alexander-held
Copy link
Member Author

Thanks for the heads-up! scikit-hep/pyhf#1560 does not interfere with this, as now the actual parameter naming format is irrelevant to cabinetry (no more magic needed to match the old MINOS names).

@alexander-held alexander-held changed the title feat: adopt pyhf parameter label API fix: compatibility with pyhf 0.6.3 (parameter label API, auxdata) Aug 26, 2021
@alexander-held alexander-held force-pushed the fix/pyhf-parameter-names branch 5 times, most recently from 20b189e to 094088b Compare August 31, 2021 11:51
@alexander-held alexander-held force-pushed the fix/pyhf-parameter-names branch from 094088b to a94fde5 Compare August 31, 2021 19:41
@alexander-held alexander-held mentioned this pull request Sep 2, 2021
@codecov
Copy link

codecov bot commented Sep 6, 2021

Codecov Report

Merging #248 (cd17d8d) into master (e45e89f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #248   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           19        19           
  Lines         1798      1787   -11     
  Branches       279       275    -4     
=========================================
- Hits          1798      1787   -11     
Impacted Files Coverage Δ
src/cabinetry/fit/__init__.py 100.00% <100.00%> (ø)
src/cabinetry/model_utils.py 100.00% <100.00%> (ø)

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 e45e89f...cd17d8d. Read the comment docs.

@alexander-held alexander-held force-pushed the fix/pyhf-parameter-names branch from f506cdc to 58614f5 Compare September 7, 2021 13:26
@alexander-held alexander-held merged commit 7b9c194 into master Sep 7, 2021
@alexander-held alexander-held deleted the fix/pyhf-parameter-names branch September 7, 2021 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants