FIBERSTATUS VARIABLETHRU bad for sky and stdstars

py/desispec/fiberbitmasking.py

@@ -110,7 +110,7 @@ def get_fiberbitmasked_frame_arrays(frame,bitmask=None,ivar_framemask=True,retur
 
 def get_fiberbitmask_comparison_value(kind,band):
     """Takes a string argument and returns a 32-bit integer representing the logical OR of all
-    relevant fibermask bits for that given reduction step
+    fatally bad fibermask bits for that given reduction step
 
     Args:
         kind: str : string designating which combination of bits to use based on the operation.
@@ -122,6 +122,8 @@ def get_fiberbitmask_comparison_value(kind,band):
     Returns:
         bitmask : 32 bit bitmask corresponding to the fiberbitmask of the desired kind
             in the desired cameras (bands).
+
+    if FIBERSTATUS & bitmask != 0, then that fiber should not be used
     """
     if kind.lower() == 'all':
         return get_all_fiberbitmask_with_amp(band)
@@ -141,17 +143,33 @@ def get_fiberbitmask_comparison_value(kind,band):
 
 
 def get_skysub_fiberbitmask_val(band):
-    return get_all_fiberbitmask_with_amp(band)
+    """
+    Return mask of bad FIBERSTATUS bits for selecting sky fibers,
+    i.e. fibers with these bits set should not be used for the sky model
+    """
+    return get_all_fiberbitmask_with_amp(band) | fmsk.VARIABLETHRU
 
 def get_flat_fiberbitmask_val(band):
+    """
+    Return mask of bad FIBERSTATUS bits for fiberflats
+    i.e. fibers with these bits set have a bad fiberflat and cannot be used
+    """
     return (fmsk.BROKENFIBER | fmsk.BADFIBER | fmsk.BADTRACE | fmsk.BADARC | \
             fmsk.MANYBADCOL | fmsk.MANYREJECTED )
 
 def get_fluxcalib_fiberbitmask_val(band):
+    """
+    Return mask of bad FIBERSTATUS bits that should trigger flux=ivar=0
+    instead of flux calibrating the spectra.
+    """
     return get_all_fiberbitmask_with_amp(band)
 
 def get_stdstars_fiberbitmask_val(band):
-    return get_all_fiberbitmask_with_amp(band) | fmsk.POORPOSITION
+    """
+    Return mask of bad FIBERSTATUS bits for selecting standard stars,
+    i.e. fibers with these bits set should not be used as standard stars
+    """
+    return get_all_fiberbitmask_with_amp(band) | fmsk.POORPOSITION | fmsk.VARIABLETHRU
 
 def get_all_nonamp_fiberbitmask_val():
     """Return a mask for all fatally bad FIBERSTATUS bits except BADAMPB/R/Z
@@ -161,16 +179,25 @@ def get_all_nonamp_fiberbitmask_val():
         be on a valid sky location, or even a target for RESTRICTED.
         Also does not include POORPOSITION which is bad for stdstars
         but not necessarily fatal for otherwise processing a normal fiber.
+        NEARCHARGETRAP and VARIABLETHRU are also not included since
+        they are ok for some types of processing but not others.
     """
     return (fmsk.BROKENFIBER | fmsk.MISSINGPOSITION | \
             fmsk.BADPOSITION | \
             fmsk.BADFIBER | fmsk.BADTRACE | fmsk.BADARC | fmsk.BADFLAT | \
             fmsk.MANYBADCOL | fmsk.MANYREJECTED )
 
 def get_justamps_fiberbitmask():
+    """
+    Return a mask of the amp-specific FIBERSTATUS bits
+    """
     return ( fmsk.BADAMPB | fmsk.BADAMPR | fmsk.BADAMPZ )
 
 def get_all_fiberbitmask_with_amp(band):
+    """
+    Return all fatally bad FIBERSTATUS bits including the amp-specific
+    bit for this band
+    """
     amp_mask = get_all_nonamp_fiberbitmask_val()
     if band.lower().find('b')>=0:
         amp_mask |= fmsk.BADAMPB
@@ -181,4 +208,7 @@ def get_all_fiberbitmask_with_amp(band):
     return amp_mask
 
 def get_all_fiberbitmask_val():
+    """
+    Return a mask of all fatally bad FIBERSTATUS bits
+    """
     return ( get_all_nonamp_fiberbitmask_val() | get_justamps_fiberbitmask() )

py/desispec/test/test_fiberbitmask.py

@@ -11,6 +11,8 @@
 
 from desispec.test.util import get_frame_data
 from desispec.fiberbitmasking import get_fiberbitmasked_frame_arrays
+from desispec.fiberbitmasking import get_fiberbitmask_comparison_value
+from desispec.maskbits import fibermask
 
 class TestFrameBitMask(unittest.TestCase):
 
@@ -38,3 +40,44 @@ def test_framebitmask(self):
 
         ivar2 = get_fiberbitmasked_frame_arrays(self.frame, bitmask=1)
         self.assertTrue( np.all(ivar1 == ivar2) )
+
+    def check_mask(self, bitname, ok_steps, bad_steps):
+        """
+        Check get_fiberbitmask_comparison_value(step, 'b') for every step.
+        FIBERSTATUS fibermask.mask(bitname) should be set for every step
+        in bad_steps and not set for every step in ok_steps.
+        """
+        for step in ok_steps:
+            mask = get_fiberbitmask_comparison_value(step, 'b')
+            self.assertTrue(mask & fibermask.mask(bitname) == 0, f"{step=} unnecessarily excludes {bitname}")
+
+        for step in bad_steps:
+            mask = get_fiberbitmask_comparison_value(step, 'b')
+            self.assertTrue(mask & fibermask.mask(bitname) != 0, f"{step=} should exclude {bitname} but doesn't")
+
+    def test_ambiguous_maskbits(self):
+        """Test cases that are bad for some steps but not for others
+        """
+
+        # NOTE: fiberfitmask doesn't currently support arc
+
+        #- BROKENFIBER is bad for everything
+        self.check_mask('BROKENFIBER', ok_steps=[], bad_steps=['flat', 'sky', 'stdstar', 'fluxcalib'])
+
+        #- RESTRICTED is ok for everything
+        self.check_mask('RESTRICTED', ok_steps=['flat', 'sky', 'stdstar', 'fluxcalib'], bad_steps=[])
+
+        #- BADPOSITION is ok for flats, but bad for others
+        self.check_mask('BADPOSITION', ok_steps=['flat',], bad_steps=['sky', 'stdstar', 'fluxcalib'])
+
+        #- POORPOSITION is ok for flats, sky, and fluxcalib; but bad for stdstars
+        self.check_mask('POORPOSITION', ok_steps=['flat', 'sky', 'fluxcalib'], bad_steps=['stdstar'])
+
+        #- NEARCHARGETRAP is informative; treated as ok for everyone including sky
+        #- TODO: it's actually bad for faint targets and sky for a single amp, but we structurally
+        #- don't have a way to encode that in FIBERSTATUS (fiber not CCD or amp)
+        self.check_mask('NEARCHARGETRAP', ok_steps=['flat', 'sky', 'stdstar', 'fluxcalib'], bad_steps=[])
+
+        #- VARIABLETHRU is ok for flats because otherwise we'd block the entire fiber,
+        #- and ok to at least attempt to flux calibrate it, but it shouldn't be used for sky or stdstars
+        self.check_mask('VARIABLETHRU', ok_steps=['flat', 'fluxcalib'], bad_steps=['sky', 'stdstar'])