-
Notifications
You must be signed in to change notification settings - Fork 362
Introduced a new arguement --skymap-only-ifos in pycbc_live #4346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
262ed5d
9c6fe0c
3c9ad87
a07558b
91ee29f
8191eb0
3330fb8
4f96d97
b738bee
f85a717
a309fe5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -165,12 +165,14 @@ class LiveEventManager(object): | |
return combined, data_ends[0] | ||
|
||
def compute_followup_data(self, ifos, triggers, | ||
followup_ifos=None, recalculate_ifar=False): | ||
followup_ifos=None, skymap_only_ifos=None, | ||
recalculate_ifar=False): | ||
"""Figure out which of the followup detectors are usable, and compute | ||
SNR time series for all the available detectors. | ||
""" | ||
out = {} | ||
followup_ifos = [] if followup_ifos is None else followup_ifos | ||
skymap_only_ifos = [] if skymap_only_ifos is None else skymap_only_ifos | ||
|
||
template_id = triggers[f'foreground/{ifos[0]}/template_id'] | ||
htilde = self.bank[template_id] | ||
|
@@ -207,13 +209,22 @@ class LiveEventManager(object): | |
if pvalue_info is None: | ||
continue | ||
out[ifo] = {'snr_series': pvalue_info['snr_series']} | ||
self.get_followup_info( | ||
ifos[0], | ||
ifo, | ||
triggers, | ||
pvalue_info, | ||
recalculate_ifar=recalculate_ifar | ||
) | ||
if ifo in skymap_only_ifos: | ||
self.get_followup_info( | ||
ifos[0], | ||
ifo, | ||
triggers, | ||
pvalue_info, | ||
recalculate_ifar=False | ||
GarethCabournDavies marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
else: | ||
self.get_followup_info( | ||
ifos[0], | ||
ifo, | ||
triggers, | ||
pvalue_info, | ||
recalculate_ifar=recalculate_ifar | ||
) | ||
|
||
# the SNR time series sample rate can vary slightly due to | ||
# rounding errors, so force all of them to be identical | ||
|
@@ -416,7 +427,7 @@ class LiveEventManager(object): | |
# reload_buffer time. These args are documented here: | ||
# https://ligo-gracedb.readthedocs.io/en/latest/api.html#ligo.gracedb.rest.GraceDb | ||
# Because we do not change any of the request session values when running the | ||
# code, it should remain thread safe. | ||
# code, it should remain thread safe. | ||
gdbargs = {'reload_certificate': True, 'reload_buffer': 300} | ||
if self.gracedb_server: | ||
gdbargs['service_url'] = self.gracedb_server | ||
|
@@ -443,10 +454,11 @@ class LiveEventManager(object): | |
gid | ||
) | ||
|
||
def check_coincs(self, ifos, coinc_results, psds): | ||
def check_coincs(self, ifos, coinc_results, psds, skymap_only_ifos=None): | ||
""" Perform any followup and save zerolag triggers to a coinc xml file | ||
""" | ||
# Check for hardware injection | ||
skymap_only_ifos = [] if skymap_only_ifos is None else skymap_only_ifos | ||
for ifo in ifos: | ||
if self.data_readers[ifo].near_hwinj(): | ||
coinc_results['HWINJ'] = True | ||
|
@@ -468,6 +480,7 @@ class LiveEventManager(object): | |
coinc_ifos, | ||
coinc_results, | ||
followup_ifos=followup_ifos, | ||
skymap_only_ifos=skymap_only_ifos, | ||
recalculate_ifar=True | ||
) | ||
|
||
|
@@ -528,7 +541,8 @@ class LiveEventManager(object): | |
args=thread_args) | ||
gdb_upload_thread.start() | ||
|
||
def check_singles(self, results, psds): | ||
def check_singles(self, results, psds, skymap_only_ifos=None): | ||
skymap_only_ifos = [] if skymap_only_ifos is None else skymap_only_ifos | ||
active = [k for k in results if results[k] is not None] | ||
for ifo in active: | ||
single = sngl_estimator[ifo].check( | ||
|
@@ -548,6 +562,7 @@ class LiveEventManager(object): | |
[ifo], | ||
single, | ||
followup_ifos=followup_ifos, | ||
skymap_only_ifos=skymap_only_ifos, | ||
recalculate_ifar=False | ||
) | ||
# Apply a trials factor of the number of active detectors | ||
|
@@ -910,6 +925,8 @@ parser.add_argument('--enable-embright-has-massgap', action='store_true', defaul | |
parser.add_argument('--embright-massgap-max', type=float, default=5.0, metavar='SOLAR MASSES', | ||
help='Upper limit of the mass gap, used for estimating ' | ||
'HasMassGap probability.') | ||
parser.add_argument('--skymap-only-ifos', nargs='+', default=['V1', 'K1'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small problem here that there is no way that I can see to set none of the ifos to be skymap-only. I would set the default to be []. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Gareth, all the changes you suggested have been made. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! I'll let @titodalcanton hit the approve button, in case there was anything else I missed |
||
help="IFOs that only contribute in sky localization") | ||
|
||
scheme.insert_processing_option_group(parser) | ||
LiveSingle.insert_args(parser) | ||
|
@@ -949,7 +966,9 @@ if bank.min_f_lower < args.low_frequency_cutoff: | |
'({} Hz)'.format(args.low_frequency_cutoff, bank.min_f_lower)) | ||
|
||
ifos = set(args.channel_name.keys()) | ||
skymap_only_ifos = set(args.skymap_only_ifos) | ||
logging.info('Analyzing data from detectors %s', ppdets(ifos)) | ||
logging.info('Detectors that only aid in the sky localization %s', ppdets(skymap_only_ifos)) | ||
|
||
evnt = LiveEventManager(args, bank) | ||
|
||
|
@@ -1029,7 +1048,8 @@ with ctx: | |
|
||
# Create double coincident background estimator for every combo | ||
if args.enable_background_estimation and evnt.rank == 0: | ||
ifo_combos = itertools.combinations(ifos, 2) | ||
trigg_ifos = [ifo for ifo in ifos if ifo not in skymap_only_ifos] | ||
ifo_combos = itertools.combinations(trigg_ifos, 2) | ||
estimators = [] | ||
for combo in ifo_combos: | ||
logging.info('Will calculate %s background', ppdets(combo, "-")) | ||
|
@@ -1100,7 +1120,7 @@ with ctx: | |
args.max_psd_abort_distance) | ||
status = False | ||
|
||
if status is True: | ||
if ifo not in skymap_only_ifos and status is True: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic is not really what we want, because it will print "Insufficient data for %s analysis" even if a detector is a skymap-only detector. |
||
evnt.live_detectors.add(ifo) | ||
if evnt.rank > 0: | ||
logging.info('Filtering %s', ifo) | ||
|
@@ -1160,11 +1180,11 @@ with ctx: | |
# Pick the best coinc in this chunk | ||
best_coinc = Coincer.pick_best_coinc(coinc_results) | ||
|
||
evnt.check_coincs(list(results.keys()), best_coinc, psds) | ||
evnt.check_coincs(list(results.keys()), best_coinc, psds, skymap_only_ifos=skymap_only_ifos) | ||
|
||
# Check for singles | ||
if args.enable_single_detector_background: | ||
evnt.check_singles(results, psds) | ||
evnt.check_singles(results, psds, skymap_only_ifos=skymap_only_ifos) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can simplify the code if you set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what I had initially thought, but dropped the idea seeeing that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test failure seems to be a condor installation issue - I will click to restart failed tests in case this was a transient thing, but we may need a fix elsewhere if it isnt |
||
|
||
gates = {ifo: data_reader[ifo].gate_params for ifo in data_reader} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you set the default value in kwargs to be [], and then you don't have this bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that is not a good idea. What @SouradeepPal did initially is the recommended approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference: https://www.quora.com/In-Python-Why-shouldnt-the-default-argument-be-to-make-an-empty-list