DM-10009: Ensure masks are valid from ImageMapReduceTask #63

djreiss · 2017-06-05T23:12:40Z

Ensure all pixels that are not touched by the reducer are flagged in masks

These pixels are likely to be the result of a bug in the mapper.

Also revent divide by zero when averaging results in weights of 0.

natelust · 2017-06-12T19:31:53Z

python/lsst/ip/diffim/imageMapReduce.py

@@ -275,15 +276,36 @@ def run(self, mapperResults, exposure, **kwargs):
                    wtsView = afwImage.ImageI(weights, item.getBBox())
                    wtsView.getArray()[isNotNan] += 1

+        isNan = (np.isnan(newMI.getImage().getArray()) |
+                  np.isnan(newMI.getVariance().getArray()))


Since nan * anything is also a nan, you could probably just do np.isnan(newMI.getImage().getArray() * newMI.getVariance().getArra()), and save one loop over all the pixels.

natelust · 2017-06-12T19:46:30Z

python/lsst/ip/diffim/imageMapReduce.py

@@ -275,15 +276,36 @@ def run(self, mapperResults, exposure, **kwargs):
                    wtsView = afwImage.ImageI(weights, item.getBBox())
                    wtsView.getArray()[isNotNan] += 1

+        isNan = (np.isnan(newMI.getImage().getArray()) |
+                  np.isnan(newMI.getVariance().getArray()))
+        if np.sum(isNan) > 0:


more over, since you are looping over the whole bool mask array here an again below, with [isNan], by don't you change the above to:
tmp = newMI.getImage().getArray() * newMI.getVariance().getArray()
isNan = np.where(tmp != tmp) # nan never equals anything

That is only 3 loops over the data, and then if if statement can be:
if len(isNan[0]) > 0: # len will fetch the precomputed property,
and you can use isNan[0] and isNan[1] as indicies to mask.getArray(), which will be many fewer indices to loop over as it can do direct array random access.

natelust · 2017-06-12T19:53:12Z

python/lsst/ip/diffim/imageMapReduce.py

+            self.log.info('AVERAGE: Minimum overlap: %f', wts.min())
+            wtsZero = wts == 0.
+            self.log.info('AVERAGE: Number of zero pixels: %f (%f%%)', np.sum(wtsZero),
+                          np.mean(wtsZero) * 100.)


here you are needlessly looping twice, consider doing:
wtsZeroSum = np.sum(wtsZero)
self.log.info('AVERAGE: Number of zero pixels: %f (%f%%)', wtsZeroSum,
wtsZeroSum/wtsZeroSum.size * 100.)

Also just consider using afw statistics directly, I think you can have it build and return multiple statistics all at once.

natelust · 2017-06-12T19:54:49Z

python/lsst/ip/diffim/imageMapReduce.py

-            wtsZero = wts == 0.
-            newMI.getImage().getArray()[wtsZero] = newMI.getVariance().getArray()[wtsZero] = np.nan
-            # TBD: set mask to something for pixels where wts == 0. Shouldn't happen. (DM-10009)
+            if np.sum(wtsZero) > 0:


consider moving this if statement higher, so some of the above does not need to get done if it is not true, also reuse the variable I suggested.

thought about moving it higher, but the logic doesn't work. But your second comment is valid.

natelust · 2017-06-12T19:59:18Z

python/lsst/ip/diffim/imageMapReduce.py

+                # New mask plane - for debugging map-reduced images
+                mask.addMaskPlane('INVALID_MAPREDUCE')
+                bad = mask.getPlaneBitMask(['INVALID_MAPREDUCE', 'BAD', 'NO_DATA'])
+                mask.getArray()[wtsZero] |= bad


if you combine the steps to produce a single mask, you would only need to add a mask plane and populate it once. More over I think this code will fail if np.sum(isNan) >0 and if np.sum(wtsZero) >0, as the mask plane will already exists and an exception will be thrown when trying to add it a second time.

natelust · 2017-06-12T20:13:53Z

python/lsst/ip/diffim/imageMapReduce.py

+            wtsZero = wts == 0.
+            self.log.info('AVERAGE: Number of zero pixels: %f (%f%%)', np.sum(wtsZero),
+                          np.mean(wtsZero) * 100.)
+            wts[wtsZero] = 1e-29  # avoid division by zero warnings -- will set to nan and flag them below


This line is not needed and loops over pixels needlessly, see below

natelust · 2017-06-12T20:18:42Z

python/lsst/ip/diffim/imageMapReduce.py

+            wtsZero = wts == 0.
+            self.log.info('AVERAGE: Number of zero pixels: %f (%f%%)', np.sum(wtsZero),
+                          np.mean(wtsZero) * 100.)
+            wts[wtsZero] = 1e-29  # avoid division by zero warnings -- will set to nan and flag them below
            newMI.getImage().getArray()[:, :] /= wts
            newMI.getVariance().getArray()[:, :] /= wts


numpy ufuncs save us here, you can just do:
np.divide(newMI.getImage().getArray(), wts, out=newMI.getImage().getArray(), where=~wtsZero)
This will do inplace operation (output is the same as input) and only loop over and do a calculation where wtsZero is not true, all in one step.

Only works as:

tmp = newMI.getImage().getArray() np.divide(tmp, wts, out=tmp, where=~wtsZero)

will do it this way.

natelust · 2017-06-12T20:21:03Z

python/lsst/ip/diffim/imageMapReduce.py

-            newMI.getImage().getArray()[wtsZero] = newMI.getVariance().getArray()[wtsZero] = np.nan
-            # TBD: set mask to something for pixels where wts == 0. Shouldn't happen. (DM-10009)
+            if np.sum(wtsZero) > 0:
+                newMI.getImage().getArray()[wtsZero] = newMI.getVariance().getArray()[wtsZero] = np.nan


why are you setting these values to nan here? is that not redundant info with the mask plane bits set? Do we ever want nans in an image? Doesn't that make things like afw statistics need to do more work?

Not sure about your last question. This was for the case where I used wts[wtsZero] = 1e-29 . Since you helped me get rid of that, this line can go now too.

natelust · 2017-06-12T20:32:03Z

tests/testImageMapReduce.py

+        mask = newMI.getMask()  # Now check the mask
+        self.assertGreater(mask.getMaskPlane('INVALID_MAPREDUCE'), 0)
+        maskBit = mask.getPlaneBitMask('INVALID_MAPREDUCE')
+        nMasked = np.sum((mask.getArray()[:, :] & maskBit) != 0)


Im pretty sure here the operation you want is numpy.bitwise_and, you can avoid the [:,:] in that case.

natelust · 2017-06-12T20:47:52Z

python/lsst/ip/diffim/imageMapReduce.py

@@ -261,6 +261,7 @@ def run(self, mapperResults, exposure, **kwargs):
            patchMI = item.getMaskedImage()
            isNotNan = ~(np.isnan(patchMI.getImage().getArray()) |
                         np.isnan(patchMI.getVariance().getArray()))


See my comment below about multiplying the arrays together to prevent needless loops. If you used np.isfinite instead of np.isnan you can save the inverting loop (if inf is also not a valid value).

At risk of premature optimization, I will implement everything you've recommended here.

These pixels are likely to be the result of a bug in the mapper. Prevent divide by zero when averaging results in weights of 0. Add unit test to check mask

natelust reviewed Jun 12, 2017

View reviewed changes

djreiss force-pushed the tickets/DM-10009 branch from 77806f6 to 4c14711 Compare June 16, 2017 19:29

Ensure all pixels not touched by the reducer are flagged in masks

2bb905f

These pixels are likely to be the result of a bug in the mapper. Prevent divide by zero when averaging results in weights of 0. Add unit test to check mask

djreiss force-pushed the tickets/DM-10009 branch from 4c14711 to 2bb905f Compare June 16, 2017 19:40

djreiss merged commit 9916df4 into master Jun 16, 2017

ktlim deleted the tickets/DM-10009 branch August 25, 2018 06:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DM-10009: Ensure masks are valid from ImageMapReduceTask #63

DM-10009: Ensure masks are valid from ImageMapReduceTask #63

djreiss commented Jun 5, 2017

natelust Jun 12, 2017

natelust Jun 12, 2017

natelust Jun 12, 2017

natelust Jun 12, 2017

natelust Jun 12, 2017

djreiss Jun 12, 2017

natelust Jun 12, 2017

natelust Jun 12, 2017

natelust Jun 12, 2017

djreiss Jun 12, 2017

natelust Jun 12, 2017

djreiss Jun 12, 2017

natelust Jun 12, 2017

natelust Jun 12, 2017

djreiss Jun 12, 2017

DM-10009: Ensure masks are valid from ImageMapReduceTask #63

DM-10009: Ensure masks are valid from ImageMapReduceTask #63

Conversation

djreiss commented Jun 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment