tickets/DM-15106 Use multiband classes in meas_deblender #57
Conversation
| except TypeError: | ||
| maskedImages = [maskedImages] | ||
| if not isinstance(mMaskedImage, afwImage.MultibandMaskedImage): | ||
| mMaskedImage = [mMaskedImage] |
There was a problem hiding this comment.
It is not allowed to be. The input either needs to be a MultibandMaskedImage or a MaskedImage, as stated in the docstring.
| @@ -102,7 +101,7 @@ def __init__(self, footprint, maskedImages, psfs, psffwhms, log, filters=None, | |||
| else: | |||
| avgNoise = [avgNoise] | |||
| # Now check that all of the parameters have the same number of entries | |||
| if any([len(maskedImages)!=len(p) for p in [psfs, psffwhms, avgNoise]]): | |||
| if any([len(mMaskedImage.filters)!=len(p) for p in [psfs, psffwhms, avgNoise]]): | |||
| # Create a DeblendedParent for the Footprint in every filter | ||
| self.deblendedParents = OrderedDict([]) | ||
| for n in range(self.filterCount): | ||
| for n,f in enumerate(self.filters): |
There was a problem hiding this comment.
The problem with filter is that it's a builtin. I know that some people use it in the stack but I prefer to just use f, although if it really bugs you I can change the name.
| @@ -842,8 +840,8 @@ def run(self, exposures, sources): | |||
| created by the multiband templates. | |||
| If `self.config.saveTemplates` is `False`, then this item will be None | |||
| """ | |||
| psfs = {B:exp.getPsf() for B, exp in exposures.items()} | |||
| return self.deblend(exposures, sources, psfs) | |||
| psfs = {f:mExposure[f].getPsf() for f in mExposure.filters} | |||
There was a problem hiding this comment.
filter? f always makes me think of file handle. Although in a comprehension it is pretty obvious.
There was a problem hiding this comment.
Yeah, even if you'd like me to change it in the loops, I'd prefer to keep the f in the list comprehensions.
There was a problem hiding this comment.
filter is a built-in and against coding standards anyway: https://developer.lsst.io/python/style.html#user-defined-names-should-not-shadow-python-built-in-functions. Update: I see Fred already pointed this out above.
| should be a merged catalog of the sources in each band ('deepCoadd_mergeDet'). | ||
| Keys are the names of the filters | ||
| (should be the same as `mExposure.filters`) and the values are | ||
| `lsst.afw.table.source.source.SourceCatalog`'s,which |
There was a problem hiding this comment.
Space after comma? Is there a check that they are correct names?
| self.log.info("Deblending {0} sources in {1} exposures".format(len(sources), len(bands))) | ||
| filters = mExposure.filters | ||
| mMaskedImage = afwImage.MultibandMaskedImage(filters=mExposure.filters, image=mExposure.image, | ||
| mask=mExposure.mask, variance=mExposure.variance) |
There was a problem hiding this comment.
Hmm, so there isn't a constructor for MultibandMaskedImage that takes a MultibandExposure? That seems sad. Although perhaps you don't want that because it is a narrowing conversion? Then perhaps a static method? E.g. mMaskedImage = afwImage.MultibandMaskedImage.fromMultibandExposure(mExposure)? Or a property (mMaskedImage = mExposure.multibandMaskedImage)? both would reduce the potential for swapping image/mask/variance by accident.
There was a problem hiding this comment.
I see your point. Actually, I think that MultibandMaskedImage.fromImages would actually work if a MultibandExposure is used in place of the of a MultibandMaskedImage. I'll open a ticket to rename fromImages to a more inclusive name.
| _peak = heavy[band].getPeaks()[0] | ||
| fluxParents[band].getFootprint().addPeak(_peak.getFx(), _peak.getFy(), | ||
| _peak = heavy[f].getPeaks()[0] | ||
| fluxParents[f].getFootprint().addPeak(_peak.getFx(), _peak.getFy(), |
There was a problem hiding this comment.
Your guess is as good as mine.
| weights[mask] = 0 | ||
| badPixels = mMaskedImage.mask.getPlaneBitMask(badMask) | ||
| mask = (mMaskedImage.mask.array & badPixels) | fpMask[None, :] | ||
| weights[mask>0] = 0 |
There was a problem hiding this comment.
Was this intentional? Is it actually faster?
There was a problem hiding this comment.
I don't know if it's faster, but it's possible now and I think looks cleaner than the previous version.
There was a problem hiding this comment.
weights[mask>0] is against standards anyway. Need whitespace around >.
|
|
||
| # The peak in each band will have the same SpanSet | ||
| mask = afwImage.Mask(np.array(np.sum(model, axis=0)>0, dtype=np.int32), | ||
| xy0=debResult.footprint.getBBox().getBegin()) | ||
| mask = afwImage.Mask(np.array(np.sum(model, axis=0)>0, dtype=np.int32), xy0=xy0) |
There was a problem hiding this comment.
Since you're changing this line anyway, it's good practice to make it comply with our coding standards even if the original infraction wasn't yours.
| @@ -102,7 +101,7 @@ def __init__(self, footprint, maskedImages, psfs, psffwhms, log, filters=None, | |||
| else: | |||
| avgNoise = [avgNoise] | |||
| # Now check that all of the parameters have the same number of entries | |||
| if any([len(maskedImages)!=len(p) for p in [psfs, psffwhms, avgNoise]]): | |||
| if any([len(self.filters) !=len (p) for p in [psfs, psffwhms, avgNoise]]): | |||
There was a problem hiding this comment.
Variable name maskedImages on line 108 doesn't exist. That raise will never work now.
There was a problem hiding this comment.
Also, did you mean to put the whitespace in this line before the len instead of after? :)
No description provided.