Detector and geocenter time in PyGRB results production#5337
Conversation
…tions and on detector time for background (and foreground) as this may be timeslid
pannarale
added
v28_release_branch
| 'Rec. RA', 'Rec. Dec', 'SNR', 'Chi^2', 'Null SNR'] | ||
| th.extend([ifo+' SNR' for ifo in ifos]) | ||
| th.extend([ifo+' time shift (s)' for ifo in ifos]) | ||
| th.extend([ifo+' GPS time (s)' for ifo in ifos]) |
There was a problem hiding this comment.
For complete clarity, also change "GPS time" on line 448 to "GC GPS time (s)".
| format_strings.extend(['##.##' for _ in ifos]) | ||
| format_strings.extend(['##.##' for _ in ifos]) | ||
| format_strings.extend(['##.##' for _ in ifos]) |
There was a problem hiding this comment.
Use three decimal digits for GPS times here, and for the GC time on line 483 as well, as we can time low-mass mergers to ~1 ms but not much better.
Keep two for SNR and time shift.
| 'Rec. spin1z', 'Rec. spin2z', 'Rec. RA', 'Rec. Dec', 'SNR', 'Chi^2', | ||
| 'Null SNR'] + [ifo+' SNR' for ifo in ifos] + ['BestNR'] | ||
| 'Null SNR'] + [ifo+' SNR' for ifo in ifos] + \ | ||
| [ifo+' GPS time (s)' for ifo in ifos] + ['BestNR'] |
There was a problem hiding this comment.
Same as above, also change "GPS time" to "GC GPS time (s)" on line 572.
| '##.##', '##.##', '##.##', '##.##', '##.##', '##.##'] | ||
| format_strings.extend(['##.##' for ifo in ifos]) | ||
| format_strings.extend(['##.##' for _ in ifos]) | ||
| format_strings.extend(['##.##' for _ in ifos]) |
There was a problem hiding this comment.
Same as above, use three decimals for GPS times here and on line 593.
| sites = [ifo[0] for ifo in ifos] | ||
| # Table header | ||
| # Check against pycbc_pygrb_minifollowups prior to changing any of these | ||
| # Injections are in zero-lag so no timing information at the IFOs is needed |
There was a problem hiding this comment.
Might as well do the same change "GC GPS time (s)" here.
| # When looking at a single slide in a detector use the time at the detector | ||
| # Otherwise use the time at the geocenter to look at all slides at once | ||
| if opts.slide_id != 'all': | ||
| x_key = opts.ifo + '/end_time' |
There was a problem hiding this comment.
Hmm, I see the point of this behavior, but it could be really confusing if things are not labeled clearly. Do the plot labels clearly indicate which time is being plotted?
There was a problem hiding this comment.
I introduced an x_label variable to indicate this properly.
github-project-automation
Bot
moved this from In Progress
to Changes Requested
in PyGRB Development
Co-authored-by: Tito Dal Canton <tito@dalcanton.it>
2 participants
This PR ensures that geocenter time and detector time are correctly tracked and used by PyGRB when producing:
Standard information about the request
This is an improvement in results presentation and also a bug fix [the follow up of a loud trigger performed with timeslid data should be performed at the correct time in the slid detector(s)]
This change affects: PyGRB
This change changes: result presentation / plotting