Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-38918

Inconsistent application of Astier's amatrix in brighter fatter correction

    XMLWordPrintable

Details

    • Bug
    • Status: Done
    • Resolution: Done
    • None
    • None
    • None
    • 1
    • Data Release Production
    • 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

          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.

           

          czw Christopher Waters added a comment - 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.  

          abrought will try it.

          youtsumi Yousuke Utsumi added a comment - abrought 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

          abrought Alex Broughton added a comment - 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"

          abrought Alex Broughton added a comment - 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.

          czw Christopher Waters added a comment - 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.

          People

            czw Christopher Waters
            youtsumi Yousuke Utsumi
            Andrés Alejandro Plazas Malagón
            Alex Broughton, Andrés Alejandro Plazas Malagón, Christopher Waters, Eli Rykoff, Yousuke Utsumi
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Jenkins

                No builds found.