nu-radio / NuRadioMC

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

Wrong shower direction saved in sim_shower #379

Closed boboeyen closed 1 year ago

boboeyen commented 2 years ago

In simulation.py the shower axis is saved in _create_sim_station. However, the direction of the simparticle is used, which (to my understanding) is the opposite direction pointing towards where the particle came from, not where it is moving to.

def _create_sim_shower(self):
        """
        creates a sim_shower object and saves the meta arguments such as neutrino direction, shower energy and self.input_particle[flavor]
        """
        # create NuRadioReco event structure
        self._sim_shower = NuRadioReco.framework.radio_shower.RadioShower(self._shower_ids[self._shower_index])
        # save relevant neutrino properties
        self._sim_shower[shp.zenith] = self.input_particle[simp.zenith]
        self._sim_shower[shp.azimuth] = self.input_particle[simp.azimuth]
        self._sim_shower[shp.energy] = self._fin['shower_energies'][self._shower_index]
        self._sim_shower[shp.flavor] = self.input_particle[simp.flavor]
        self._sim_shower[shp.interaction_type] = self.input_particle[simp.interaction_type]
        self._sim_shower[shp.vertex] = self.input_particle[simp.vertex]
        self._sim_shower[shp.vertex_time] = self._vertex_time
        self._sim_shower[shp.type] = self._fin['shower_type'][self._shower_index]
        # TODO direct parent does not necessarily need to be the primary in general, but full
        # interaction chain is currently not populated in the input files.
        self._sim_shower[shp.parent_id] = self.primary.get_id()
sjoerd-bouma commented 2 years ago

Is this really a bug? According to the (ancient, so may be out of date) wiki, directions should always be defined as pointing towards their source (which I agree is somewhat unintuitive for the shower axis). I can't remember any, but do you know if this convention is followed in other cases in NuRadioMC?

cg-laser commented 2 years ago

@sjoerd-bouma thanks for looking this up. It is not a bug and rather something we need to document/define better. The shower axis that is used in the code points into the direction of propagation (note the minus sign): self._shower_axis = -1 * hp.spherical_to_cartesian(self._fin['zeniths'][self._shower_index], self._fin['azimuths'][self._shower_index]). I first thought it would be good to therefore save the zenith/azimuth angle also into the direction of the axis, but this would mix multiple definitions. I think we should stay with what you found in the wiki that we define zenith/azimuth always to "where things are coming from".

fschlueter commented 2 years ago

In Auger the following terminology is used: Axes are pointing into the direction of origin. Directions point into the direction of movement. I am not sure if that terminology makes sense here and if we want to enforce it.

To this issue: I think it can be closed? The zenith and azimuth angle are defines as pointing into "where it came from"

fschlueter commented 1 year ago

Can this be closed, or do we need to discuss this further?

In my opinion is this wrong:

self._shower_axis = -1 * hp.spherical_to_cartesian(self._fin['zeniths'][self._shower_index], self._fin['azimuths'][self._shower_index])

The variable should rather be called direction than shower_axis. Note, that the function get_axis in base_shower.py does not have this sign!

cg-laser commented 1 year ago

I tend to agree with you that we could unify the definition and use the same naming convention as Auger does. This would require to rename this variable to _shower_direction. But maybe it would be better to just leave if like this for now as this will result in conflicts with @christophwelling work on refactoring simulation.py

christophwelling commented 1 year ago

I also agree that we should try to find a common definition, but I would like to first get the refactoring of simulation.py done.

cg-laser commented 1 year ago

then I close it for now