Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-8691

ConvolvedFluxPlugin should measure even if seeing is small

    Details

      Description

      The ConvolvedFluxPlugin currently refuses to measure a source if the target seeing is smaller than the actual seeing. This was a bad choice, as having a potentially-incorrect measurement is more useful than having nothing at all (so long as there's a flag indicating the problem).

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            Hsin-Fang Chiang, would you mind reviewing this, please?

            price@price-laptop:~/LSST/meas/extensions/convolved (tickets/DM-8691=) $ git sub-patch
            commit 7f3a1913c61c08672424996048608b444396afc7
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Tue Dec 20 10:54:38 2016 -0500
             
                when deconvolving, record that fact but measure anyway
                
                This provides a more useful value than NaN.
             
            diff --git a/python/lsst/meas/extensions/convolved/convolved.py b/python/lsst/meas/extensions/convolved/convolved.py
            index e69ec5e..98fb2f1 100644
            --- a/python/lsst/meas/extensions/convolved/convolved.py
            +++ b/python/lsst/meas/extensions/convolved/convolved.py
            @@ -321,8 +321,9 @@ class BaseConvolvedFluxPlugin(lsst.meas.base.BaseMeasurementPlugin):
                             convolved = self.convolve(exposure, seeing, target/SIGMA_TO_FWHM, measRecord.getFootprint(),
                                                       maxRadius)
                         except (DeconvolutionError, RuntimeError):
            +                # Record the problem, but allow the measurement to run in case it's useful
                             measRecord.set(self.data[ii].deconvKey, True)
            -                continue
            +                convolved = exposure
                         self.measureAperture(measRecord, convolved, self.data[ii].aperture)
                         if kron is not None:
                             self.measureForcedKron(measRecord, self.data[ii].kronKeys, convolved.getMaskedImage(), kron)
            diff --git a/tests/test_convolved.py b/tests/test_convolved.py
            index 32c41af..ae9151a 100755
            --- a/tests/test_convolved.py
            +++ b/tests/test_convolved.py
            @@ -177,12 +177,11 @@ class ConvolvedFluxTestCase(lsst.utils.tests.TestCase):
                         kronRadius = source.get("ext_photometryKron_KronFlux_radius")
             
                     self.assertFalse(source.get(algName + "_flag"))  # algorithm succeeded
            -        for ii, seeing in enumerate(algConfig.seeing):
            -            deconvolve = seeing < psfFwhm/scale.asArcseconds()
            -            self.assertTrue(source.get(algName + "_%d_deconv" % ii) == deconvolve)
            -            if deconvolve:
            -                # Not worth checking anything else
            -                continue
            +        originalSeeing = psfFwhm/scale.asArcseconds()
            +        for ii, targetSeeing in enumerate(algConfig.seeing):
            +            deconvolve = targetSeeing < originalSeeing
            +            self.assertEqual(source.get(algName + "_%d_deconv" % ii), deconvolve)
            +            seeing = targetSeeing if not deconvolve else originalSeeing
             
                         def expected(radius, sigma=seeing/SIGMA_TO_FWHM):
                             """Return expected flux for 2D Gaussian with nominated sigma"""
            @@ -190,7 +189,7 @@ class ConvolvedFluxTestCase(lsst.utils.tests.TestCase):
             
                         # Kron succeeded and match expectation
                         if not forced:
            -                kronName = algConfig.getKronResultName(seeing)
            +                kronName = algConfig.getKronResultName(targetSeeing)
                             kronApRadius = algConfig.kronRadiusForFlux*kronRadius
                             self.assertClose(source.get(kronName + "_flux"), expected(kronApRadius), rtol=1.0e-3)
                             self.assertGreater(source.get(kronName + "_fluxSigma"), 0)
            @@ -198,7 +197,7 @@ class ConvolvedFluxTestCase(lsst.utils.tests.TestCase):
             
                         # Aperture measurements suceeded and match expectation
                         for jj, radius in enumerate(measConfig.algorithms[algName].aperture.radii):
            -                name = algConfig.getApertureResultName(seeing, radius)
            +                name = algConfig.getApertureResultName(targetSeeing, radius)
                             self.assertClose(source.get(name + "_flux"), expected(radius), rtol=1.0e-3)
                             self.assertFalse(source.get(name + "_flag"))
                             self.assertGreater(source.get(name + "_fluxSigma"), 0)
            

            Show
            price Paul Price added a comment - Hsin-Fang Chiang , would you mind reviewing this, please? price@price-laptop:~/LSST/meas/extensions/convolved (tickets/DM-8691=) $ git sub-patch commit 7f3a1913c61c08672424996048608b444396afc7 Author: Paul Price <price@astro.princeton.edu> Date: Tue Dec 20 10:54:38 2016 -0500   when deconvolving, record that fact but measure anyway This provides a more useful value than NaN.   diff --git a/python/lsst/meas/extensions/convolved/convolved.py b/python/lsst/meas/extensions/convolved/convolved.py index e69ec5e..98fb2f1 100644 --- a/python/lsst/meas/extensions/convolved/convolved.py +++ b/python/lsst/meas/extensions/convolved/convolved.py @@ -321,8 +321,9 @@ class BaseConvolvedFluxPlugin(lsst.meas.base.BaseMeasurementPlugin): convolved = self.convolve(exposure, seeing, target/SIGMA_TO_FWHM, measRecord.getFootprint(), maxRadius) except (DeconvolutionError, RuntimeError): + # Record the problem, but allow the measurement to run in case it's useful measRecord.set(self.data[ii].deconvKey, True) - continue + convolved = exposure self.measureAperture(measRecord, convolved, self.data[ii].aperture) if kron is not None: self.measureForcedKron(measRecord, self.data[ii].kronKeys, convolved.getMaskedImage(), kron) diff --git a/tests/test_convolved.py b/tests/test_convolved.py index 32c41af..ae9151a 100755 --- a/tests/test_convolved.py +++ b/tests/test_convolved.py @@ -177,12 +177,11 @@ class ConvolvedFluxTestCase(lsst.utils.tests.TestCase): kronRadius = source.get("ext_photometryKron_KronFlux_radius") self.assertFalse(source.get(algName + "_flag")) # algorithm succeeded - for ii, seeing in enumerate(algConfig.seeing): - deconvolve = seeing < psfFwhm/scale.asArcseconds() - self.assertTrue(source.get(algName + "_%d_deconv" % ii) == deconvolve) - if deconvolve: - # Not worth checking anything else - continue + originalSeeing = psfFwhm/scale.asArcseconds() + for ii, targetSeeing in enumerate(algConfig.seeing): + deconvolve = targetSeeing < originalSeeing + self.assertEqual(source.get(algName + "_%d_deconv" % ii), deconvolve) + seeing = targetSeeing if not deconvolve else originalSeeing def expected(radius, sigma=seeing/SIGMA_TO_FWHM): """Return expected flux for 2D Gaussian with nominated sigma""" @@ -190,7 +189,7 @@ class ConvolvedFluxTestCase(lsst.utils.tests.TestCase): # Kron succeeded and match expectation if not forced: - kronName = algConfig.getKronResultName(seeing) + kronName = algConfig.getKronResultName(targetSeeing) kronApRadius = algConfig.kronRadiusForFlux*kronRadius self.assertClose(source.get(kronName + "_flux"), expected(kronApRadius), rtol=1.0e-3) self.assertGreater(source.get(kronName + "_fluxSigma"), 0) @@ -198,7 +197,7 @@ class ConvolvedFluxTestCase(lsst.utils.tests.TestCase): # Aperture measurements suceeded and match expectation for jj, radius in enumerate(measConfig.algorithms[algName].aperture.radii): - name = algConfig.getApertureResultName(seeing, radius) + name = algConfig.getApertureResultName(targetSeeing, radius) self.assertClose(source.get(name + "_flux"), expected(radius), rtol=1.0e-3) self.assertFalse(source.get(name + "_flag")) self.assertGreater(source.get(name + "_fluxSigma"), 0)
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Looks good to me

            Show
            hchiang2 Hsin-Fang Chiang added a comment - Looks good to me
            Hide
            price Paul Price added a comment -

            Thanks Hsin-Fang Chiang!

            Merged to master.

            Show
            price Paul Price added a comment - Thanks Hsin-Fang Chiang ! Merged to master.

              People

              • Assignee:
                price Paul Price
                Reporter:
                price Paul Price
                Reviewers:
                Hsin-Fang Chiang
                Watchers:
                Hsin-Fang Chiang, Paul Price
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel