Add details of closest triggers to the injection minifollowup #4503
Conversation
|
Want to see an example of this ... Running it now. |
| trigger_times = {} | ||
| for trig in single_triggers: | ||
| ifo = trig.ifo | ||
| with h5py.File(trig.lfn, 'r') as trig_f: |
There was a problem hiding this comment.
I think this will hit the same issue as noted in #4510
I will implement something similar to the fix there in this
There was a problem hiding this comment.
Actually this won't be as bad as we will be using the injection HDF_TRIGGER_MERGE files rather than FULL_DATA, but should still be fixed
There was a problem hiding this comment.
It's less important here, as the injection TRIGGER_MERGE are an order of magnitude smaller than the full_data one ... but still, avoiding having all trigger times read into memory, when it's not needed, could be useful. It would likely slow the code down a bit though, hopefully only a small amount.
There was a problem hiding this comment.
I've added a method to find the triggers in a +/- 2s window around the missed injections, and not to display triggers outwith the window.
It might be good to cut this default to 1s maybe
|
I updated the suggestion to just |
| # But only ones which are within a small window of the missed injection | ||
| missed_inj_times = numpy.sort(f['injections/tc'][:][missed]) | ||
|
|
||
| # Note: Adding 2 * Earth diameter in light seconds to the window here |
There was a problem hiding this comment.
Should it be 2 diameters or just 1, given that you have a left and right edge?
In fact, if the injection stated end time is at geocentre, should it not rather be Earth radius?
I guess if we're not totally clear about the geocentre then 1* diameter on each side is 'safe'
There was a problem hiding this comment.
That's the point, this bit is more about cutting out things which aren't nearby - two earth diameters is probably bigger than really necessary, but is basically to help ensure that the number of triggers is manageable
There was a problem hiding this comment.
Are you saying that more than 1 diameter on either side is ever needed?
If not we should change to that
There was a problem hiding this comment.
I have updated to 1 diameter
| Trigger times to be checked against the missed injection times | ||
|
|
||
| snr: numpy array | ||
| (Unused, but to prevent a TypeError) SNR of the triggers |
There was a problem hiding this comment.
This is cryptic (very unclear why a TypeError might arise). I am guessing that the way the .select method is set up requires this. If so, rephrase eg 'Required by design of the HFile.select method but not used'
|
LGTM thanks! |
Co-authored-by: Ian Harry <[email protected]>
…where closest trigger is far away from the injection
GarethCabournDavies
force-pushed
the
inj_followup_closest_triggers
branch
from
c38702c to
f9b8b44
Compare
…o#4503) * Add details of closest triggers (not just event) to the injection minifollowup * Dont know how this was deleted * Wrap titles with quotes Co-authored-by: Ian Harry <[email protected]> * Use existing functions to reduce the number of trigger times being stored * Let the max window be set as a command line argument, deal with case where closest trigger is far away from the injection * remove import I thought I would use but didnt * Use the loudest SNR event in a tiny window instead of simply closest time * missed re-setting the ifo in this loop * Apply suggestions from code review * TD comment --------- Co-authored-by: Ian Harry <[email protected]>
…o#4503) * Add details of closest triggers (not just event) to the injection minifollowup * Dont know how this was deleted * Wrap titles with quotes Co-authored-by: Ian Harry <[email protected]> * Use existing functions to reduce the number of trigger times being stored * Let the max window be set as a command line argument, deal with case where closest trigger is far away from the injection * remove import I thought I would use but didnt * Use the loudest SNR event in a tiny window instead of simply closest time * missed re-setting the ifo in this loop * Apply suggestions from code review * TD comment --------- Co-authored-by: Ian Harry <[email protected]>
…o#4503) * Add details of closest triggers (not just event) to the injection minifollowup * Dont know how this was deleted * Wrap titles with quotes Co-authored-by: Ian Harry <[email protected]> * Use existing functions to reduce the number of trigger times being stored * Let the max window be set as a command line argument, deal with case where closest trigger is far away from the injection * remove import I thought I would use but didnt * Use the loudest SNR event in a tiny window instead of simply closest time * missed re-setting the ifo in this loop * Apply suggestions from code review * TD comment --------- Co-authored-by: Ian Harry <[email protected]>
…o#4503) * Add details of closest triggers (not just event) to the injection minifollowup * Dont know how this was deleted * Wrap titles with quotes Co-authored-by: Ian Harry <[email protected]> * Use existing functions to reduce the number of trigger times being stored * Let the max window be set as a command line argument, deal with case where closest trigger is far away from the injection * remove import I thought I would use but didnt * Use the loudest SNR event in a tiny window instead of simply closest time * missed re-setting the ifo in this loop * Apply suggestions from code review * TD comment --------- Co-authored-by: Ian Harry <[email protected]>
…o#4503) * Add details of closest triggers (not just event) to the injection minifollowup * Dont know how this was deleted * Wrap titles with quotes Co-authored-by: Ian Harry <[email protected]> * Use existing functions to reduce the number of trigger times being stored * Let the max window be set as a command line argument, deal with case where closest trigger is far away from the injection * remove import I thought I would use but didnt * Use the loudest SNR event in a tiny window instead of simply closest time * missed re-setting the ifo in this loop * Apply suggestions from code review * TD comment --------- Co-authored-by: Ian Harry <[email protected]>
The injection minifollowups currently show the closest event, this means that anything which had a trigger at the same time, but was discarded for some reason, is not obvious.
For example, this should help to diagnose why things are present in the SNR triggers plot, but not the reweighted SNR plot