Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: None
-
Labels:None
-
Story Points:1
-
Epic Link:
-
Team:Data Release Production
-
Urgent?:No
Description
The current implementation of the Brighter Fatter correction takes amatrix from the full covariance fit as-is (they are all positive). However, because of inconsistency in the sign, it doesn't match the current implementation of Coulton's BF correction algorithm (it assumes negative). When it applies the correction, the sign of amatrix needs to be flipped by a -> -a.
I don't know where this fix should go though.
Attachments
Activity
Alex Broughton will try it.
I can confirm that Chris's fix works in flipping the A matrix and results in a proper correction. But while it does work, I don’t know if it is just a fix for an issue elsewhere
Here is a plot of the correction with the negative sign change, showing that it is correcting in the "right direction"
I'll push this change through the review process, and I'll try to work through the math today and tomorrow to see if this fix is hiding other mistakes.
If it's just a simple minus sign, then it probably just needs to be added here: https://github.com/lsst/cp_pipe/blob/main/python/lsst/cp/pipe/makeBrighterFatterKernel.py#L304
As that's a trivial change, I've made it on a ticket branch here. If you can test it out to confirm that it fixes the issue, I'll make a PR and get it reviewed etc.
I thought that I had generated a similar-looking kernel with both the mean cross-correlation and A-matrix paths, but you're doing a much more in-depth study that I did.