Fix ara trigger #161
Fix ara trigger #161
Conversation
…o secondary channels from now on. Corrected a bug when checking for the positions of secondary channels.
…d noise parameters
| else: | ||
| trigger.set_triggered(True) | ||
| trigger.set_trigger_time(trigger_time_sample / sampling_rate) | ||
| trigger.set_trigger_time(trigger_time_sample) |
There was a problem hiding this comment.
you might want to rename the variable from trigger_time_sample to trigger_time. Otherwise its confusing.
I'm running some tests on the cluster, but it has been taken by the usual suspects and my jobs won't run. I might postpone merging until the tests are complete and we understand them. |
| if trigger[channel_id]: | ||
| times = channel.get_times() | ||
| trace_after_diode = self.tunnel_diode(channel) | ||
| arg_trigger = np.argwhere( trace_after_diode == np.min(trace_after_diode) )[0,0] |
There was a problem hiding this comment.
Isn't this equivalent to just writing arg_trigger=np.argmin(trace_after_diode)?
There was a problem hiding this comment.
You're right. I've changed it.
| if coinc >= number_concidences: | ||
| trigger_time = None | ||
|
|
||
| trace_times = station.get_channel(0).get_times() |
There was a problem hiding this comment.
There may not be a channel with ID 0.
As addressed in #160 , I don't think we should assume that all channels have the same trace_start_time, since there may be differences between the deep and shallow channels or if we have a double trigger for DnR
There was a problem hiding this comment.
That was in the module before. Let me change it.
There was a problem hiding this comment.
Keep in mind that the assumption that they have the same trace_start_time is nowhere. I get, for every channel, the correct trigger times array given by the get_times() method. I agree that the way of sliding the window is not the best, as it should go from the minimum time of all the channels, to the maximum time.
There was a problem hiding this comment.
Done. Let me know if it's better this way.
… the maximum times from all the channels.
…ime. If the number of triggers is less than the required coincidences, the coincidence window is not calculated
|
Here's a test of the effective volume: phased array vs ARA trigger. The phased array performs better at low energies, as expected, and if I had used equal bandwidths, I would have had a better improvement. However, the diode trigger has an increase at high energies for 100 m, and I don't understand why. Maybe it's a product of the non-linearity of the diode? No idea. I'm going to check if that depth has been calculated properly. |
|
Ok, proof that the new ARA trigger works and it's worse than a phased array trigger. Simulations have been run with a large bandwidth. I'm going to run some simulations for 140-530 MHz, but I think the addressed changes and the checks are complete. @cg-laser @christophwelling do you approve the current version or do I need to change more things? |
|
For a restricted band (140-530 MHz), the trigger is even better. Remember that this simulation has been made with the buggy arrival directions, but it is a fair trigger comparison nevertheless. |
While the tunnel diode implementation seems to work just fine, the diode trigger for each channel was later ignored when calculating the coincidences. I've implemented a simpler method for calculating the coincidences - the code takes the minimum of the diode-filtered trace and stores the corresponding time as the trigger time. Then, using a sliding window, it checks if a requested number of coincidences lies in that window. I've also added a new method for the trigger class that calculates the threshold parameters power_mean and power_std using the bandwidth and noise RMS as input.