DM-35175: Fix the variance plane calculation when the science image is convolved #222
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kherner
approved these changes
Jun 24, 2022
tests/test_subtractTask.py
Outdated
|
|
||
| # Verify that the variance plane of the difference image is correct | ||
| # when the template and science variance planes are incorrect | ||
| science.variance.array[...] = scienceVarianceOrig/scaleFactor | ||
| # science.variance.array[...] = scienceVarianceOrig/scaleFactor |
There was a problem hiding this comment.
Just checking this is intentionally commented out.
There was a problem hiding this comment.
I was able to fix the test, so that this commented-out code could be put back in.
| @@ -326,22 +345,29 @@ def runConvolveScience(self, template, science, sources): | |||
| # We must invert the background model if the matching kernel is solved for the science image. | |||
| kernelResult.backgroundModel.setParameters([-p for p in modelParams]) | |||
|
|
|||
| kernelImage = lsst.afw.image.ImageD(kernelResult.psfMatchingKernel.getDimensions()) | |||
| norm = kernelResult.psfMatchingKernel.computeImage(kernelImage, doNormalize=False) | |||
There was a problem hiding this comment.
Is there any reason one might want to allow doNormalize to be configurable here, as opposed to hard-coded false? I think not but doesn't hurt to think about it a bit.
There was a problem hiding this comment.
It's actually important that it be hard-coded as False here, so that we get the flux scale right.
Set convolutionControl to copy edge pixels instead of producing NaNs Increase accuracy of model variance plane for tests Add test coverage of variance plane when science image is convolved Put normalization factor on its own line Set the photometric scale and photoCalib when convolving the science image. Return the matchedScience image. Add a warning when running in compatibility mode Scale the science and template variance first instead of the diffim later. This makes the ratio between the science and template variances correct when the images are subtracted. SubtractTask tests must assume the inputs can be modified. This is because ScaleVariance now operates on the science and template images. Use afw.math statistics to compute image and variance means Simplify import Add test of ScaleVariance metadata fix flake8 errors Clean up scaleVariance unit test from review
isullivan
force-pushed
the
tickets/DM-35175
branch
from
9ce76d5
to
aa08fde
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.