cta-observatory/pyirf

Wrong normalization of EDISP

Closed this issue · 7 comments

maxnoe commented

GADF states that EDISP needs to be a PDF (i.e. the integral should be 1), we compute a probability for each bin (i.e. the sum is 1).

I am not sure what this as for consequences e.g. for the LST analysis and how large of a difference this makes, to be checked.

CC @chaimain, @moralejo

maxnoe commented

Just checked the DL3 releases in the joint crab of MAGIC, VERITAS and HESS and they use the pdf definition. Will check the public CTA IRFs soon, but I doubt they'll be different.

FACT's are normalized like here, which is where we inherited that bug from.

kosack commented

It reminds me this needs to be written as an interface requirements for DPPS-SUSS

It is not easy to test this with the current lstchain v0.10.x (#1141), as we need an updated MC production from lstmcpipe.

Once we have the new production, testing just the Edisp normalization on say, the LST analysis of Crab Nebula data, can be done without needing to update lstchain's dependency on pyirf (which should be done in the near future) and maybe we will see some minor difference.

maxnoe commented

@chaimain I am testing it now, will write an email soon. The existing dl3 files can easily be fixed without any recomputation.

Thanks @maxnoe. Does the EDISP have units attached? (for the "z" axis) - that would make it clear it is a probability density and not a bin-integrated probability. In one of the MAGIC programs for spectral calculations we use bin-integrated probabilities, with very fine Etrue bins, and it is of course not a problem if you treat them as such in the steps afterwards.

maxnoe commented

@moralejo no, it does not, as the axis has no dimension

$$\mu = \frac{E_\text{est}}{E_\text{true}}$$

And it's a conventional choice if you store probabilities or probability densities, but of course it makes a difference if you use the wrong convention.

maxnoe commented

Closed by merging #250