nu-radio / NuRadioMC

A Monte Carlo simulation package for radio neutrino detectors
GNU General Public License v3.0
33 stars 35 forks source link

Incorrect phasing in the noise-gen script #699

Closed colemanalan closed 2 months ago

colemanalan commented 2 months ago

https://github.com/nu-radio/NuRadioMC/blob/19e807e04a4aa3ddeda5e06c4008a94f15f3ec37/NuRadioReco/utilities/noise.py#L105-L110

This part of the for loop doesn't actually do what we want. Both if-statements do the same roll so that a delay of e.g. 5 is the same as -5.

Example traces[i] == [0, 1, 2, 3, 4, 5]

if r = +2, we want output to be [4, 5, 0, 1, 2, 3]

if r = -2, we want output to be [2, 3, 4, 5, 0, 1]

I guess the direction that things get moved for positive values of r. But I don't think that we want both positive and negative delays should give the same result.


For comparison, if we use numpy's roll, one gets

In [1]: np.roll([i for i in range(6)], 2)
Out[1]: array([4, 5, 0, 1, 2, 3])
In [2]: np.roll([i for i in range(6)], -2)
Out[2]: array([2, 3, 4, 5, 0, 1])

Since we want to use the switch at https://github.com/nu-radio/NuRadioMC/blob/19e807e04a4aa3ddeda5e06c4008a94f15f3ec37/NuRadioReco/utilities/noise.py#L533-L537 it is probably preferred that we use numpy def (also what is used in the actual PA trigger module for simulations). So we could fix this by removing the elif loop and define r = -rolling[i] to get the same result.