-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
Add DecorrelateALKernelTask as a subtask #63
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,10 +36,9 @@ | |
from lsst.ip.diffim import ImagePsfMatchTask, DipoleAnalysis, \ | ||
SourceFlagChecker, KernelCandidateF, cast_KernelCandidateF, makeKernelBasisList, \ | ||
KernelCandidateQa, DiaCatalogSourceSelectorTask, DiaCatalogSourceSelectorConfig, \ | ||
GetCoaddAsTemplateTask, GetCalexpAsTemplateTask, DipoleFitTask | ||
GetCoaddAsTemplateTask, GetCalexpAsTemplateTask, DipoleFitTask, DecorrelateALKernelTask | ||
import lsst.ip.diffim.diffimTools as diffimTools | ||
import lsst.ip.diffim.utils as diUtils | ||
from lsst.ip.diffim.imageDecorrelation import decorrelateExposure | ||
|
||
FwhmPerSigma = 2 * math.sqrt(2 * math.log(2)) | ||
IqrToSigma = 0.741 | ||
|
@@ -67,7 +66,7 @@ class ImageDifferenceConfig(pexConfig.Config): | |
"Ignored if doPreConvolve false.") | ||
doDetection = pexConfig.Field(dtype=bool, default=True, doc="Detect sources?") | ||
doDecorrelation = pexConfig.Field(dtype=bool, default=False, | ||
doc="Perform diffim decorrelation to undo pixel correlation due to convolution? " | ||
doc="Perform diffim decorrelation to undo pixel correlation due to A&L kernel convolution? " | ||
"If True, also update the diffim PSF.") | ||
doMerge = pexConfig.Field(dtype=bool, default=True, | ||
doc="Merge positive and negative diaSources with grow radius set by growFootprint") | ||
|
@@ -108,6 +107,10 @@ class ImageDifferenceConfig(pexConfig.Config): | |
target=ImagePsfMatchTask, | ||
doc="Warp and PSF match template to exposure, then subtract", | ||
) | ||
decorrelate = pexConfig.ConfigurableField( | ||
target=DecorrelateALKernelTask, | ||
doc="Decorrelate effects of A&L kernel convolution on image difference, only if doSubtract is True", | ||
) | ||
detection = pexConfig.ConfigurableField( | ||
target=SourceDetectionTask, | ||
doc="Low-threshold detection for final measurement", | ||
|
@@ -167,6 +170,10 @@ def setDefaults(self): | |
# DiaSource Detection | ||
self.detection.thresholdPolarity = "both" | ||
self.detection.thresholdValue = 5.5 | ||
# If decorrelation is turned on, no need to "hack" the detection threshold to decrease the number of | ||
# false detections, so set it back to 5.0 | ||
if self.doDecorrelation and self.doSubtract: | ||
self.detection.thresholdValue = 5.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a comment explaining why you change this value. Also, since you only run decorrelation if doSubtract is True, I suggest you apply the same test here. That makes the "doCorrelate" config parameter a bit easier to explain (e.g. "ignored if doSubtract False") There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good points. Done and done. |
||
self.detection.reEstimateBackground = False | ||
self.detection.thresholdType = "pixel_stdev" | ||
|
||
|
@@ -212,6 +219,7 @@ def __init__(self, butler=None, **kwargs): | |
pipeBase.CmdLineTask.__init__(self, **kwargs) | ||
self.makeSubtask("subtract") | ||
self.makeSubtask("getTemplate") | ||
self.makeSubtask("decorrelate") | ||
|
||
if self.config.doUseRegister: | ||
self.makeSubtask("register") | ||
|
@@ -516,10 +524,13 @@ def run(self, sensorRef, templateIdList=None): | |
template = self.getTemplate.run(exposure, sensorRef, templateIdList=templateIdList) | ||
subtractedExposure.setPsf(template.exposure.getPsf()) | ||
|
||
# If doSubtract is False, then subtractedExposure was fetched from disk (above), thus it may have | ||
# already been decorrelated. Thus, we do not do decorrelation if doSubtract is False. | ||
if self.config.doDecorrelation and self.config.doSubtract: | ||
self.log.info("Running diffim decorrelation; updating diffim PSF.") | ||
subtractedExposure, _ = decorrelateExposure(templateExposure, exposure, subtractedExposure, | ||
subtractRes.psfMatchingKernel, self.log) | ||
decorrResult = self.decorrelate.run(templateExposure, exposure, | ||
subtractedExposure, | ||
subtractRes.psfMatchingKernel) | ||
subtractedExposure = decorrResult.correctedExposure | ||
|
||
if self.config.doDetection: | ||
self.log.info("Running diaSource detection") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment noting that it will be ignored if doSubtract is False (other than affecting the default value of detection.thresholdValue, which I think should be fixed).
Also, a brief explanation of WHY you only decorrelate if doSubtract is true would be nice. I assume it is because there is no noise to decorrelate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you actually mean a comment? Or add it to the docstring? I will add a comment.
My understanding is that if doSubtract is False, then the actual image differencing is not done, and instead a saved image difference is loaded from disk. I am considering image decorrelation as being "part of" image subtraction so if no subtraction performed, then no decorrelation. This assumption may eventually need to be changed, but if so then we need to somehow store the fact that the diffim was decorrelated in its fits header. For now I think it's reasonable to assume that decorrelation will have been run prior to saving.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant please add the explanation it to the doc string. In general if a config field needs explanation, that explanation should go into the doc string.