codecov[bot]
commented
2 years ago Codecov Report
Merging #324 (d41065f) into master (f34c73a) will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## master #324 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 23
Lines 1889 1886 -3
Branches 306 305 -1
=========================================
- Hits 1889 1886 -3 | Impacted Files | Coverage Δ | |
|---|---|---|
| src/cabinetry/model_utils.py | 100.00% <ø> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update f34c73a...d41065f. Read the comment docs.
The
model_utils.yield_stdevimplementation contained an optimization forstaterror-staterrorcross terms, which have no contribution when calculating per-bin uncertainties: https://github.com/scikit-hep/cabinetry/blob/f34c73acec8d80e012ff0a4abbf44b29204af888/src/cabinetry/model_utils.py#L303-L307When the functionality to calculate per-channel uncertainties was added in #189, the optimization should have been updated, since these cross terms no longer cancel out for the per-channel calculation. This now fixes the bug in per-channel yield uncertainties by removing this performance optimization completely.
While this does make the calculation slower overall, that is not a concern since it will be replaced completely by #316 afterwards, resulting in a significant speed-up. This intermediate fix is just meant to split the development up into steps that are hopefully easier to follow when looking back at this in the future.
resolves #323