From fbd0113023839aac5d54e6b940c4616e31b4fe2b Mon Sep 17 00:00:00 2001
From: Brandon Piotrzkowski <piotrzk3@uwm.edu>
Date: Sat, 28 Dec 2019 17:42:52 -0500
Subject: [PATCH] Fix spacetime coincidence FAR

---
 gwcelery/tasks/raven.py            | 48 +++++++++-----------------
 gwcelery/tests/test_tasks_raven.py | 55 +++++++++++-------------------
 2 files changed, 36 insertions(+), 67 deletions(-)

diff --git a/gwcelery/tasks/raven.py b/gwcelery/tasks/raven.py
index a61f78e8e..04e69d786 100644
--- a/gwcelery/tasks/raven.py
+++ b/gwcelery/tasks/raven.py
@@ -36,19 +36,6 @@ def calculate_coincidence_far(superevent, exttrig, group):
     if exttrig['pipeline'] == 'SNEWS':
         return
 
-    #  Try to grab superevent and external sky maps
-    try:
-        se_skymap = external_skymaps.get_preferred_skymap(
-            superevent_id)
-    except ValueError:
-        se_skymap = None
-    try:
-        ext_skymap = gracedb.download('glg_healpix_all_bn_v00.fit',
-                                      exttrig_id)
-    except HTTPError as e:
-        if e.status == 404:
-            ext_skymap = None
-
     tl_cbc, th_cbc = app.conf['raven_coincidence_windows']['GRB_CBC']
     tl_burst, th_burst = app.conf['raven_coincidence_windows']['GRB_Burst']
 
@@ -56,25 +43,24 @@ def calculate_coincidence_far(superevent, exttrig, group):
         tl, th = tl_cbc, th_cbc
     elif group == 'Burst':
         tl, th = tl_burst, th_burst
-
-    if ext_skymap and se_skymap:
-        return calc_signif(exttrig['search'],
-                           superevent_id, exttrig_id, tl, th,
-                           incl_sky=True,
-                           se_fitsfile=se_skymap)
+     
+    if {'LUMIN_GO', 'SKYMAP_READY'}.issubset(exttrig['labels']):
+        #  if both sky maps available, calculate spat coinc far
+        se_skymap = external_skymaps.get_preferred_skymap(
+            superevent_id)
+        ext_skymap = external_skymaps.get_external_skymap_filename(
+            exttrig_id)
+
+        return ligo.raven.search.calc_signif_gracedb(
+                   superevent_id, exttrig_id, tl, th,
+                   grb_search=exttrig['search'],
+                   se_fitsfile=se_skymap, ext_fitsfile=ext_skymap,
+                   incl_sky=True, gracedb=gracedb.client)
     else:
-        return calc_signif(exttrig['search'],
-                           superevent_id, exttrig_id, tl, th,
-                           incl_sky=False)
-
-
-@app.task(shared=False)
-def calc_signif(search, se_id, exttrig_id, tl, th, incl_sky=False,
-                se_fitsfile=None):
-    """Calculate FAR of GRB exttrig-GW coincidence"""
-    return ligo.raven.search.calc_signif_gracedb(
-        se_id, exttrig_id, tl, th, grb_search=search, se_fitsfile=se_fitsfile,
-        incl_sky=incl_sky, gracedb=gracedb.client)
+         return ligo.raven.search.calc_signif_gracedb(
+                   superevent_id, exttrig_id, tl, th,
+                   grb_search=exttrig['search'],
+                   incl_sky=False, gracedb=gracedb.client)
 
 
 @app.task(shared=False)
diff --git a/gwcelery/tests/test_tasks_raven.py b/gwcelery/tests/test_tasks_raven.py
index 0f4f9e23c..dbbc731bb 100644
--- a/gwcelery/tests/test_tasks_raven.py
+++ b/gwcelery/tests/test_tasks_raven.py
@@ -62,67 +62,50 @@ def test_raven_search(mock_raven_search, mock_se_cls, mock_exttrig_cls,
 
 
 @pytest.mark.parametrize('group', ['CBC', 'Burst'])
-@patch('gwcelery.tasks.raven.calc_signif')
+@patch('ligo.raven.search.calc_signif_gracedb')
 def test_calculate_coincidence_far(
         mock_calc_signif, group):
     se = {'superevent_id': 'S1234'}
     ext = {'graceid': 'E4321',
            'pipeline': 'Fermi',
-           'search': 'GRB'}
+           'search': 'GRB',
+           'labels': []}
     raven.calculate_coincidence_far(se, ext, group)
     if group == 'CBC':
         tl, th = -5, 1
     else:
         tl, th = -600, 60
-    mock_calc_signif.assert_called_once_with('GRB', 'S1234',
-                                             'E4321', tl, th,
-                                             incl_sky=False)
+    mock_calc_signif.assert_called_once_with(
+        'S1234', 'E4321', tl, th,
+        incl_sky=False, grb_search='GRB',
+        gracedb=gracedb.client)
 
 
 @pytest.mark.parametrize('group', ['CBC', 'Burst'])  # noqa: F811
+@patch('gwcelery.tasks.external_skymaps.get_external_skymap_filename',
+       return_value='fermi_skymap.fits.gz')
 @patch('gwcelery.tasks.external_skymaps.get_preferred_skymap',
        return_value='bayestar.fits.gz')
-@patch('gwcelery.tasks.raven.calc_signif')
+@patch('ligo.raven.search.calc_signif_gracedb')
 def test_calculate_spacetime_coincidence_far(
-        mock_calc_signif, mock_get_preferred_skymap, group):
+        mock_calc_signif, mock_get_preferred_skymap,
+        mock_get_external_skymap_filename, group):
     se = {'superevent_id': 'S1234'}
     ext = {'graceid': 'E4321',
            'pipeline': 'Fermi',
-           'search': 'GRB'}
+           'search': 'GRB',
+           'labels': ['LUMIN_GO', 'SKYMAP_READY']}
     raven.calculate_coincidence_far(se, ext, group)
     if group == 'CBC':
         tl, th = -5, 1
     else:
         tl, th = -600, 60
-    mock_calc_signif.assert_called_once_with('GRB', 'S1234',
-                                             'E4321', tl, th,
-                                             incl_sky=True,
-                                             se_fitsfile='bayestar.fits.gz')
-
-
-@patch('ligo.raven.search.calc_signif_gracedb')
-def test_calc_signif(
-        mock_raven_calc_signif):
-    tl, th = -1, 5
-    raven.calc_signif('GRB', 'S1234', 'E1234', tl, th,
-                      incl_sky=False)
-
-    mock_raven_calc_signif.assert_called_once_with(
-        'S1234', 'E1234', tl, th, grb_search='GRB',
-        se_fitsfile=None, incl_sky=False,
-        gracedb=gracedb.client)
-
-
-@patch('ligo.raven.search.calc_signif_gracedb')
-def test_calc_signif_skymaps(mock_raven_calc_signif):
-    tl, th = -1, 5
-    raven.calc_signif('GRB', 'S1234', 'E1234', tl, th,
-                      incl_sky=True, se_fitsfile='bayestar.fits.gz')
-
-    mock_raven_calc_signif.assert_called_once_with(
-        'S1234', 'E1234', tl, th, grb_search='GRB',
+    mock_calc_signif.assert_called_once_with(
+        'S1234', 'E4321', tl, th,
+        incl_sky=True, grb_search='GRB',
         se_fitsfile='bayestar.fits.gz',
-        incl_sky=True, gracedb=gracedb.client)
+        ext_fitsfile='fermi_skymap.fits.gz',
+        gracedb=gracedb.client)
 
 
 def mock_get_labels(superevent_id):
-- 
GitLab