Skip to content

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

Merged
merged 11 commits into from
Jul 11, 2023
28 changes: 18 additions & 10 deletions bin/pycbc_live
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ 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=[],
recalculate_ifar=False):
"""Figure out which of the followup detectors are usable, and compute
SNR time series for all the available detectors.
"""
Expand Down Expand Up @@ -212,7 +213,7 @@ class LiveEventManager(object):
ifo,
triggers,
pvalue_info,
recalculate_ifar=recalculate_ifar
recalculate_ifar=recalculate_ifar and ifo not in skymap_only_ifos
)

# the SNR time series sample rate can vary slightly due to
Expand Down Expand Up @@ -416,7 +417,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
Expand All @@ -443,7 +444,7 @@ class LiveEventManager(object):
gid
)

def check_coincs(self, ifos, coinc_results, psds):
def check_coincs(self, ifos, coinc_results, psds, skymap_only_ifos=[]):
""" Perform any followup and save zerolag triggers to a coinc xml file
"""
# Check for hardware injection
Expand All @@ -468,6 +469,7 @@ class LiveEventManager(object):
coinc_ifos,
coinc_results,
followup_ifos=followup_ifos,
skymap_only_ifos=skymap_only_ifos,
recalculate_ifar=True
)

Expand Down Expand Up @@ -503,7 +505,7 @@ class LiveEventManager(object):
ifar = coinc_results['foreground/ifar']
upload_checks = self.enable_gracedb_upload and self.ifar_upload_threshold < ifar
optimize_snr_checks = self.run_snr_optimization and self.ifar_upload_threshold < ifar

# Keep track of the last few coincs uploaded in order to
# prevent singles being uploaded as well for coinc events
self.last_few_coincs_uploaded.append(event.merger_time)
Expand All @@ -528,7 +530,7 @@ 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=[]):
active = [k for k in results if results[k] is not None]
for ifo in active:
single = sngl_estimator[ifo].check(
Expand All @@ -548,6 +550,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
Expand Down Expand Up @@ -910,6 +913,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=[],
help="IFOs that only contribute in sky localization")
Copy link
Contributor

@titodalcanton titodalcanton May 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not set defaults here. Also, replace "IFOs" with "detectors" in the help string.


scheme.insert_processing_option_group(parser)
LiveSingle.insert_args(parser)
Expand Down Expand Up @@ -949,7 +954,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 = list(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)

Expand Down Expand Up @@ -1029,7 +1036,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, "-"))
Expand Down Expand Up @@ -1100,7 +1108,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:
Copy link
Contributor

Choose a reason for hiding this comment

The 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)
Expand Down Expand Up @@ -1160,11 +1168,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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can simplify the code if you set skymap_only_ifos to be an attribute of the LiveEventManager class. Then you do not need to carry around all these arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 ifos is not an attribute of the LiveEventManager. Is there a reason. Now I have added skymap_only_ifos as an attribute which seems to have simplified things. But the test failed which I cannot understand atm. Where should I look at to debug.

Copy link
Contributor

Choose a reason for hiding this comment

The 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}

Expand Down