Make AcqProbs ref to parent acqs be a weakref

proseco/__init__.py

@@ -1,4 +1,4 @@
-__version__ = "4.3.1"
+__version__ = "4.3.2"
 
 
 def get_aca_catalog(*args, **kwargs):

proseco/acq.py

@@ -6,6 +6,8 @@
 https://docs.google.com/presentation/d/1VtFKAW9he2vWIQAnb6unpK4u1bVAVziIdX9TnqRS3a8
 """
 
+import weakref
+
 import numpy as np
 from scipy import ndimage, stats
 from scipy.interpolate import interp1d
@@ -126,6 +128,23 @@ class AcqTable(ACACatalogTable):
     _fid_set = MetaAttribute(is_kwarg=False, default=())
     imposters_mag_limit = MetaAttribute(is_kwarg=False, default=20.0)
 
+    def __setstate__(self, state):
+        """Set self during unpickling.
+        This has special handling to deal with restoring the ``acqs`` weak
+        reference in the AcqProbs objects.  Since weakrefs cannot be pickled,
+        they are simply dropped prior to pickling and restored here.
+        """
+        super().__setstate__(state)
+
+        # This could be a cand_acqs table or acqs table, so check if
+        # ``cand_acqs`` has something, and if so then create the weakref in
+        # each of the AcqProbs objects stored in the ``probs`` column.  TO DO:
+        # make two separate classes AcqTable and CandAcqTable to avoid this
+        # contextual hack.
+        if self.cand_acqs is not None:
+            for probs in self.cand_acqs['probs']:
+                probs.acqs = weakref.ref(self)
+
     @classmethod
     def empty(cls):
         """
@@ -1142,7 +1161,7 @@ def __init__(self, acqs, acq, dither, stars, dark, t_ccd, date):
         self._p_fid_spoiler = {}
         self._p_fid_id_spoiler = {}
 
-        self.acqs = acqs
+        self.acqs = weakref.ref(acqs)
 
         # Convert table row to plain dict for persistence
         self.acq = {key: acq[key] for key in ('yang', 'zang')}
@@ -1185,7 +1204,7 @@ def p_acq_model(self, box_size):
         return self._p_acq_model[box_size]
 
     def p_acqs(self, box_size, man_err):
-        fid_set = self.acqs.fid_set
+        fid_set = self.acqs().fid_set
         try:
             return self._p_acqs[box_size, man_err, fid_set]
         except KeyError:
@@ -1197,12 +1216,12 @@ def p_acqs(self, box_size, man_err):
             return p_acq
 
     def p_acq_marg(self, box_size):
-        fid_set = self.acqs.fid_set
+        fid_set = self.acqs().fid_set
         try:
             return self._p_acq_marg[box_size, fid_set]
         except KeyError:
             p_acq_marg = 0.0
-            for man_err, p_man_err in zip(CHAR.man_errs, self.acqs.p_man_errs):
+            for man_err, p_man_err in zip(CHAR.man_errs, self.acqs().p_man_errs):
                 p_acq_marg += self.p_acqs(box_size, man_err) * p_man_err
             self._p_acq_marg[box_size, fid_set] = p_acq_marg
             return p_acq_marg
@@ -1219,7 +1238,7 @@ def p_fid_spoiler(self, box_size):
         :param box_size: search box size in arcsec
         :returns: probability multiplier (0 or 1)
         """
-        fid_set = self.acqs.fid_set
+        fid_set = self.acqs().fid_set
         try:
             return self._p_fid_spoiler[box_size, fid_set]
         except KeyError:
@@ -1246,10 +1265,10 @@ def p_fid_id_spoiler(self, box_size, fid_id):
         try:
             return self._p_fid_id_spoiler[box_size, fid_id]
         except KeyError:
-            fids = self.acqs.fids
+            fids = self.acqs().fids
             if fids is None:
-                self.acqs.add_warning('Requested fid spoiler probability without '
-                                      'setting acqs.fids first')
+                self.acqs().add_warning('Requested fid spoiler probability without '
+                                        'setting acqs.fids first')
                 return 1.0
 
             p_fid_id_spoiler = 1.0
@@ -1258,8 +1277,9 @@ def p_fid_id_spoiler(self, box_size, fid_id):
             except (KeyError, IndexError, AssertionError):
                 # This should not happen, but ignore with a warning in any case.  Non-candidate
                 # fid cannot spoil an acq star.
-                self.acqs.add_warning(f'Requested fid spoiler probability for fid '
-                                      f'{self.acqs.detector}-{fid_id} but it is not a candidate')
+                self.acqs().add_warning(f'Requested fid spoiler probability for fid '
+                                        f'{self.acqs().detector}-{fid_id} but it is not '
+                                        f'a candidate')
             else:
                 if fids.spoils(fid, self.acq, box_size):
                     p_fid_id_spoiler = 0.0
@@ -1268,6 +1288,15 @@ def p_fid_id_spoiler(self, box_size, fid_id):
 
             return p_fid_id_spoiler
 
+    def __getstate__(self):
+        """Get the state object for pickling.
+        Normally this self.__dict__, but for this class we need to drop the ``acqs``
+        attribute which is a weakref and cannot be pickled.
+        """
+        state = self.__dict__.copy()
+        del state['acqs']
+        return state
+
 
 def get_p_man_err(man_err, man_angle):
     """

proseco/core.py

@@ -679,6 +679,17 @@ def __getstate__(self):
 
         return columns, meta
 
+    def __setstate__(self, state):
+        """Restore object from pickle state.
+        This fixes an upstream issue in astropy.table (as of 3.1) where the
+        Table __setstate__ does ``self.__init__(columns, meta)``, which makes a
+        deepcopy of ``meta``.  That is inefficient but more importantly does
+        not preserve the ``acqs`` weakref in cand_acqs['probs'].
+        """
+        columns, meta = state
+        self.__init__(columns)
+        self.meta.update(meta)
+
     def to_pickle(self, rootdir='.'):
         """
         Write the catalog table as pickle to:

proseco/tests/test_catalog.py

@@ -155,6 +155,17 @@ def test_big_dither_from_mica_starcheck():
 
 
 def test_pickle():
+    """Test that ACA, guide, acq, and fid catalogs round-trip through pickling.
+
+    Known attributes that do NOT round-trip are below.  None of these are
+    required for post-facto catalog evaluation and currently the reporting code
+    handles ``stars`` and ``dark``.
+
+    - stars
+    - dark
+    - aca.fids.acqs
+
+    """
     stars = StarsTable.empty()
     stars.add_fake_constellation(mag=10.0, n_stars=5)
     aca = get_aca_catalog(stars=stars, raise_exc=True, **STD_INFO)
@@ -188,6 +199,14 @@ def test_pickle():
             else:
                 assert val == val2
 
+    # Test that calc_p_safe() gives the same answer, which implicitly tests
+    # that the AcqTable.__setstate__ unpickling code has the right (weak)
+    # reference to acqs within each AcqProbs object.  This also tests
+    # that acqs.p_man_err and acqs.fid_set are the same.
+    assert np.isclose(aca.acqs.calc_p_safe(), aca2.acqs.calc_p_safe(),
+                      atol=0, rtol=1e-6)
+    assert aca.acqs.fid_set == aca2.acqs.fid_set
+
 
 def test_big_sim_offset():
     """