Fixing the FrequentistCalculator module

[tmombaecher]

I tried the FrequentistCalculator for limit setting with this nice library, but unfortunately found it creating quite weird results, as also reported in #175.
So yesterday I threw myself at it and managed to get a working version with some tweaks.
It may not be most elegant python nor do I have the overview whether this fix is not breaking another use case. But it seems to work for me. I attach the script I use for testing (effectively adapted from https://github.com/scikit-hep/hepstats/blob/main/notebooks/hypotests/upperlimit_freq_zfit.ipynb):
demo_example.zip

The fix addresses several points:

1. the change in the frequentist_calculator module: it's actually a fix on the theory level: to get the poinull test statistic distribution, you will want to evaluate the test statistic at the same point at which you generate (this is different for poialt). In https://arxiv.org/pdf/1007.1727 poinull is denoted as $\mu$, while poialt is denoted with $\mu^\prime$

2. the change on the other files have the following aim:
   The observation was that the toys thrown were always the same and corresponding to the original fit result given to the calculator. This is wrong. One wants the toys to correspond to the profile fit at the poigen value. So I had to
   a) set the starting parameters instead of the bestfit to the profile fit at the poigen value
   b) request that the distribution is thrown from a new loss every time

I put some plots up here, made with the demo example that I attached, which demonstrate that the fix works.
clscurves.pdf
massfit.pdf
q-distributions.pdf

[jonas-eschle]

Hi @tmombaecher many thanks for looking at it and submitting a PR! This slipped my attention, I'll have a look through tomorrow, but this looks all reasonable. I wonder why this was done like this in the first place...

[nsahoo]

@jonas-eschle Is there any plan to get this PR merged in near future ? We are gonna be using this fix for our analysis.

[eduardo-rodrigues]

Hi @nsahoo, @tmombaecher, I updated the branch and approved the CI to test. I will rely of course on @jonas-eschle as package maintainer for more. One thing you should do is fix the pre-commit ...

One other little suggestion: I reckon some of your info above, at least the arXiv reference, would be good to have with the code so that anyone later one can understand better.

[jonas-eschle]

Hi @nsahoo apologies for the silence! I worked on it, and it seems to not be quite right, something is off, so I am trying to figure out what it is. I.e. other tests give some weird results. But on it, will update you!

[nsahoo]

Thanks @jonas-eschle, looking forward to the update! If you don't mind, may I ask what is off ?

[nsahoo]

Hi @jonas-eschle, any further update on this?