-
Notifications
You must be signed in to change notification settings - Fork 1
DM-53465: Fix PSF normalization #171
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
Conversation
| warping_kernel_name, | ||
| ) | ||
| else: | ||
| psf_padding = 10 |
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.
The only other non-lanczos option is bilinear; why 10 for that and only 5 for lanczos3?
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.
5 for Lanczos3 is well-motivated because that is the support of the kernel and verified that making it 6 or 7 made no difference to the output. I didn't want to make any strong assumptions about the support of non-Lanczos kernels and took a more conservative approach of 10. I also didn't want to introduce it as a config option.
| psf_stacker.add_masked_image(warped_psf_maskedImage, weight=weight) | ||
|
|
||
| if warped_psf_maskedImage.image.array.sum() < 0.995: | ||
| if not (0.995 < (psf_normalization := warped_psf_maskedImage.image.array.sum()) < 1.005): |
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 think at some point this needs to actually do something rather than warn, like try to increase the padding and then fall back to renormalizing by the sum (which should only happen if the PSF cut off, i.e. >0 at the edge).
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.
Maybe, but for now, I just want to monitor this to make sure this never happens. Piff guarantees that the model it returns is normalized and lanczos resampling shouldn't introduce deviations that are larger than 0.005. If the PSF normalization is off by more than 0.5%, it means something with the assumptions made is incorrect and I'd like to know that before padding is auto-adjusted.
c01bc23 to
fa23d1f
Compare
fa23d1f to
386051a
Compare
This PR addresses a bug with PSF not being normalized to unity in cell-based coadds.