Harvard-Neutrino / prometheus

GNU Lesser General Public License v2.1
16 stars 8 forks source link

`initial_state_azimuth` appears to be in range `[-pi,pi]` #40

Open RasmusOrsoe opened 4 months ago

RasmusOrsoe commented 4 months ago

Dear Prometheus,

The initial_state_azimuth appears to be in range [-pi,pi] but I think most people would expect it to fall in the range of [0, 2pi]. (At least I was surprised).

When we create 3D-direction vectors from these angles, it does make a difference how the angle is projected (specifically the sin(ø) is not invariant under this projection). This could cause confusion and bad labels.

I would suggest that all azimuthal angles from the simulation is outputted in the [0,2pi] projection instead.

jlazar17 commented 3 months ago

I don't see how $\sin\phi$ is not invariant. For example

import numpy as np
a = np.random.uniform(-np.pi, np.pi, 100)
aprime = np.where(a<0, a+2 * np.pi, a)
np.sum(np.abs(np.sin(a) - np.sin(aprime)))

gives 0. Am I missing something ?

This being said, I am happy to modify the output to be in the requested range.

RasmusOrsoe commented 3 months ago

Hey @jlazar17 - thanks for commenting!

Here's the check I made to verify that it's not invariant.

image

The user can obviously handle this. But if the user expects a certain range and blindly converts the angles to direction vectors, one might not realize that this is an issue (that's what happened to me).

jlazar17 commented 3 months ago

This is not the transformation that you need to perform ! You need to shift by $2\pi$.

RasmusOrsoe commented 3 months ago

This is not the transformation that you need to perform ! You need to shift by 2π.

Yes exactly. My point is just (perhaps poorly explained above) that it would be convenient for the angle to be shifted by 2pi by default, to avoid surprises.

jlazar17 commented 3 months ago

Wait, sorry, I am still a bit confused about what the surprise here is. Of course $\sin(\phi)!=\sin(-\phi)$ in general, but $\sin(\phi) = \sin(\phi + 2\pi)$ so I am missing how this would cause issues.

In any case, I can make it so that the output range is within the input range so that the user can specify the branch they are interested in.