fix: Keep staterror from affecting multiple samples

[lukasheinrich]

Pull Request Description

• Resolves Large cls value difference between v0.6.3 and master #1720
• Resolves staterror regressions blocking v0.7.0 #1945

This PR is supposed to fix the issue w/ staterrors which erroneously affected multiple samples.

• Would be great to get add. test coverage and cross-checks w/ real world models

@alexander-held @matthewfeickert @kratsg

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

* Fix staterror bug introduced after v0.6.3 where staterror erroneously affected
  multiple samples.
   - Amends PR #1639 which introduced regression.
   - c.f. Issue #1720
* Remove staterror_combined.__staterror_uncrt as it is unused by the codebase.
* Add test to verify that staterror does not affect more samples than shapesys equivalently.

Co-authored-by: Giordon Stark <[email protected]>

[matthewfeickert]

Would be great to get add. test coverage and cross-checks w/ real world models

👍 From the list in Issue #1945, we should be able to check against Issue #1720.

[codecov]

Codecov Report

Merging #1965 (3bacc51) into master (d4e1fc2) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1965      +/-   ##
==========================================
- Coverage   98.24%   98.24%   -0.01%     
==========================================
  Files          68       68              
  Lines        4380     4377       -3     
  Branches      728      726       -2     
==========================================
- Hits         4303     4300       -3     
  Misses         45       45              
  Partials       32       32              

Flag	Coverage Δ
contrib	26.57% <ø> (+0.01%)	⬆️
doctest	60.56% <ø> (+0.01%)	⬆️
unittests-3.10	96.13% <ø> (-0.01%)	⬇️
unittests-3.7	96.12% <ø> (-0.01%)	⬇️
unittests-3.8	96.16% <ø> (-0.01%)	⬇️
unittests-3.9	96.18% <ø> (-0.01%)	⬇️

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

Impacted Files	Coverage Δ
src/pyhf/modifiers/staterror.py	98.03% <ø> (-0.06%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[alexander-held]

Thanks for this fix! Works as intended in the setups I have tested and I did not spot any problems with it.

[matthewfeickert]

Thanks @lukasheinrich for this PR and to @kratsg for adding the very nice test! I just have some questions that hopefully have very easy answers that I'm just missing as it is late for me.

(@kratsg As I was the one who originally moved this PR to draft I moved it to ready for review. So we were waiting for me and not Lukas I think.)

[matthewfeickert]

LGTM. 👍 Thanks @lukasheinrich and @kratsg!

Late tonight after I'm done packing I'll try to get Issue #1907 resolved so that we can do a v0.7.0rc2 correctly and then I'll send out a request email for testing and being drafting up the v0.7.0 release notes in PR #1705 so that we can release v0.7.0 early this week.