New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DM-30465 Implement decorrelation afterburner for maximum likelihood images #197

Merged

isullivan merged 1 commit into master from tickets/DM-30465

Sep 25, 2021

Member

kgabor commented Aug 13, 2021

Add preConvMode argument to task run.
Add likelihood image correction for the image.
Add variance plane estimation for the likelihood image.
Use new variance plane estimation for the difference image.
Add full variance plane propagation.
Add support for Inf/NaN values in variance plane handling.
Add preConvKernel, preConvMode consistency check at run entry point.
Default preconvolution kernel to getLocalKernel for average psf position.
Add unit test of new task config option.

isullivan approved these changes

View reviewed changes

Contributor

isullivan left a comment

I left comments on a few typos and missing documentation. Otherwise, the main change is that I think we always want to do the full variance plane calculation, rather than the estimation. Aside from speed concerns, is there a case when the estimate would be more robust or otherwise preferable?

python/lsst/ip/diffim/imageDecorrelation.py Outdated

Comment on lines 51 to 56

+                  completeVarPlanePropagation = pexConfig.Field(
+                      dtype=bool,
+                      default=False,
+                      doc="Compute the full effect of the decorrelated matching kernel on the variance plane."
+                          " Otherwise use a model weighed sum of the input variances."
+                  )

Contributor

isullivan Aug 17, 2021

Is there ever a case when we would not want to do the full computation? I'm thinking this should at least default to True, if not be required (instead of being a config option).

python/lsst/ip/diffim/imageDecorrelation.py Outdated

-                          if not None, then the `scienceExposure` was pre-convolved with this kernel.
-                          Allowed only if ``templateMatched==True``.
+                          If not `None`, then the `scienceExposure` was pre-convolved with (the reflection of)
+                          this kernel. Must be normed to sum 1.

Contributor

isullivan Aug 17, 2021

"normalized" instead of "normed"

python/lsst/ip/diffim/imageDecorrelation.py Outdated

                       Parameters
                       ----------
                       scienceExposure : `lsst.afw.image.Exposure`
-                          The original science exposure (before `preConvKernel` applied).
+                          The original science exposure (before pre-convolution, if ``preConvMode==True``).
                       templateExposure : `lsst.afw.image.Exposure`
                           The original template exposure warped into the science exposure dimensions.
                       subtractedExposure : `lsst.afw.iamge.Exposure`

Contributor

isullivan Aug 17, 2021

Typo: "iamge"

Contributor

isullivan Aug 17, 2021

Also, include a description of this parameter when preConvMode=True

python/lsst/ip/diffim/imageDecorrelation.py Outdated

-                      spatially fixed PSF. It is calculated as the center of image PSF corrected by the center of
-                      image matching kernel.
+                      If ``preConvMode==True``, ``subtractedExposure`` is assumed to be a
+                      score image and a the noise correction for likelihood images

Contributor

isullivan Aug 17, 2021

Typo: "and a the..."

python/lsst/ip/diffim/imageDecorrelation.py

+                      If ``preConvMode==True``, ``subtractedExposure`` is assumed to be a
+                      score image and a the noise correction for likelihood images
+                      is applied. The resulting image is an optimal detection likelihood image
+                      when the templateExposure has noise. (See DMTN-179) If ``preConvKernel`` is

Contributor

isullivan Aug 17, 2021

Add double backticks around templateExposure

python/lsst/ip/diffim/imageDecorrelation.py Outdated

+                      when the templateExposure has noise. (See DMTN-179) If ``preConvKernel`` is
+                      not specified, the PSF of ``scienceExposure`` is assumed as pre-convolution kernel.
+                      The `subtractedExposure` is NOT updated. The returned `correctedExposure`

Contributor

isullivan Aug 17, 2021

Use double backticks here as well

python/lsst/ip/diffim/imageDecorrelation.py

+                          if preConvKernel is None:
+                              preConvKernel = scienceExposure.getPsf().getLocalKernel()  # at average position
+                          preConvImg = afwImage.ImageD(preConvKernel.getDimensions())
+                          preConvKernel.computeImage(preConvImg, True)

Contributor

isullivan Aug 17, 2021

Please add a code comment above this line to briefly describe the operation.

python/lsst/ip/diffim/imageDecorrelation.py Outdated

-                      # Allow for numpy type casting
-                      varImg[...] = preSum*preSum*exposure.variance.array + kSumSq*matchedExposure.variance.array
+                      if self.config.completeVarPlanePropagation:
+                          self.log.debug("Complete variance plane calculation")

Contributor

isullivan Aug 17, 2021

I think these log messages need to be raised to info, and re-worded to be more meaningful in the logs.

python/lsst/ip/diffim/imageDecorrelation.py Outdated

+                              exposure.variance.array, matchedExposure.variance.array,
+                              expVar, matchedVar, corr.cnft, corr.crft)
+                      else:
+                          self.log.debug("Estimation of variance plane")

Contributor

isullivan Aug 17, 2021

Raise to info

python/lsst/ip/diffim/imageDecorrelation.py

@@ @@ -682,7 +839,7 @@ def computeVarianceMean(self, exposure): @@
                       return var
                   def run(self, scienceExposure, templateExposure, subtractedExposure, psfMatchingKernel,
-                          spatiallyVarying=True, preConvKernel=None, templateMatched=True):
+                          spatiallyVarying=True, preConvKernel=None, templateMatched=True, preConvMode=False):

Contributor

isullivan Aug 17, 2021

Add the definition of preConvMode to the docstring


          Implement likelihood image correction in the decorrelation afterburner.

c5ab107

 * Add preConvMode argument to task run.
 * Add likelihood image correction for the image.
 * Add variance plane estimation for the likelihood image.
 * Use new variance plane estimation for the difference image.
 * Add full variance plane propagation.
 * Add support for Inf/NaN values in variance plane handling.
 * Add preConvKernel, preConvMode consistency check at run entry point.
 * Default preconvolution kernel to getLocalKernel for average psf position.
 * Add unit test of new task config option.

isullivan force-pushed the tickets/DM-30465 branch from 6a30304 to c5ab107 Compare

September 25, 2021 00:59

isullivan merged commit fa60afe into master

isullivan deleted the tickets/DM-30465 branch

September 25, 2021 18:10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment