diff --git a/proseco/__init__.py b/proseco/__init__.py index 57b01cfc..542bd5c3 100644 --- a/proseco/__init__.py +++ b/proseco/__init__.py @@ -1,4 +1,4 @@ -__version__ = "4.3.1" +__version__ = "4.3.2" def get_aca_catalog(*args, **kwargs): diff --git a/proseco/acq.py b/proseco/acq.py index e069e65b..94ed9056 100644 --- a/proseco/acq.py +++ b/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): """ diff --git a/proseco/core.py b/proseco/core.py index b6102b3b..518c44c8 100644 --- a/proseco/core.py +++ b/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: diff --git a/proseco/tests/test_catalog.py b/proseco/tests/test_catalog.py index 7586d077..b54edcc1 100644 --- a/proseco/tests/test_catalog.py +++ b/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(): """