DM-52599: refactor kernel source selection in subtractImages #435
Conversation
BrunoSanchez
left a comment
BrunoSanchez
left a comment
There was a problem hiding this comment.
I think I understand the changes and they improve the flow of the kernel source selection.
I asked two questions just for my better understanding but they shouldn't be a blocker on merging the code, specially if tests pass just fine.
| kernelResult = self.runMakeKernel(template, science, kernelSources, | ||
| convolveTemplate=convolveTemplate) |
There was a problem hiding this comment.
I have a question: here self.runMakeKernel is attempting to run the self.makeKernel.run and if it fails it checks for the same checkTemplateIsSufficient.
I see here a nested logic that is basically adding complexity but it repeats itself? I don't quite follow why.
There was a problem hiding this comment.
Good catch! The duplication of error handling is unnecessary. I can rearrange the new code a little to remove it, and the run method will be simpler and cleaner.
tests/test_subtractTask.py
Outdated
| # get the img psf Noise Equivalent Area value | ||
| nea = computePSFNoiseEquivalentArea(science.psf) | ||
| self.assertFloatsAlmostEqual(stdVal, np.sqrt(2)*noiseLevel/np.sqrt(nea), rtol=0.1) | ||
| self.assertFloatsAlmostEqual(stdVal, np.sqrt(2)*noiseLevel/np.sqrt(nea), rtol=0.2) |
There was a problem hiding this comment.
Why is the relative tolerance now needing an adjustment? I wonder if the PSF now is less exact?
There was a problem hiding this comment.
The issue is that slightly more pixels are excluded near the edge during preconvolution, which I think is appropriate. That means that the background model that is calculated as part of the kernel fit is slightly different, but I think that is OK since we don't use this background model anyway - the real background subtraction is in detectAndMeasure. The change is not as large as my commit made it seem, I will reduce the tolerance to a value closer to the original.
This also prepares the selected sources to calculate the kernel by adding consistent footprints to each source.
isullivan
force-pushed
the
tickets/DM-52599
branch
from
b57ea83 to
ef14210
Compare
2 participants
This moves the final steps of kernel source selection from the
makeKernelsubtask to the_sourceSelectormethod ofsubtractImages, which allows for earlier and cleaner error handling when there are too few kernel candidates. It also allows some of the duplicated methods from the different flavors of image subtraction to be removed, since they can now directly use the methods of the base class.