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

MegaCam vertical overscan dimensions off by one

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: obs_cfht
    • Labels:
      None
    • Story Points:
      0.25
    • Sprint:
      DRP X16-1
    • Team:
      Data Release Production

      Description

      Dominique Boutigny discovered, following the merge of new functionality in ISR that masks the vertical overscan, that the vertical overscan extent is too large by one pixel for all CCDs. They should be 1024x32 instead of 1024x33.

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            price@lsst-dev:~/LSST/obs/cfht (tickets/DM-5524) $ git sub
            commit e81cfda6a4e9c5ab382d91cfb47c9cfc4255649e
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Thu Mar 17 09:12:26 2016 -0500
             
                fix megacam vertical overscan
                
                The vertical overscan was too large by a single pixel, for all CCDs.
                
                Edited via:
                
                    path = "megacam/camera/ccd%02d.fits"
                    for ii in range(36):
                        ccd = afwTable.AmpInfoCatalog.readFits(path % ii)
                        for amp in ccd:
                            bbox = amp.getRawVerticalOverscanBBox()
                            amp.setRawVerticalOverscanBBox(afwGeom.Box2I(bbox.getMin(),
                                bbox.getDimensions() - afwGeom.Extent2I(1, 1)))
                        ccd.writeFits(path % ii)
             
             megacam/camera/ccd00.fits |    4 ++--
             megacam/camera/ccd01.fits |    2 +-
             megacam/camera/ccd02.fits |    2 +-
             megacam/camera/ccd03.fits |    2 +-
             megacam/camera/ccd04.fits |    2 +-
             megacam/camera/ccd05.fits |    2 +-
             megacam/camera/ccd06.fits |    4 ++--
             megacam/camera/ccd07.fits |    2 +-
             megacam/camera/ccd08.fits |    2 +-
             megacam/camera/ccd09.fits |    2 +-
             megacam/camera/ccd10.fits |    2 +-
             megacam/camera/ccd11.fits |    2 +-
             megacam/camera/ccd12.fits |    4 ++--
             megacam/camera/ccd13.fits |    2 +-
             megacam/camera/ccd14.fits |    4 ++--
             megacam/camera/ccd15.fits |    4 ++--
             megacam/camera/ccd16.fits |    4 ++--
             megacam/camera/ccd17.fits |    2 +-
             megacam/camera/ccd18.fits |    4 ++--
             megacam/camera/ccd19.fits |    4 ++--
             megacam/camera/ccd20.fits |    2 +-
             megacam/camera/ccd21.fits |    4 ++--
             megacam/camera/ccd22.fits |    4 ++--
             megacam/camera/ccd23.fits |    2 +-
             megacam/camera/ccd24.fits |    4 ++--
             megacam/camera/ccd25.fits |    2 +-
             megacam/camera/ccd26.fits |    6 +++---
             megacam/camera/ccd27.fits |    2 +-
             megacam/camera/ccd28.fits |    4 ++--
             megacam/camera/ccd29.fits |    2 +-
             megacam/camera/ccd30.fits |    2 +-
             megacam/camera/ccd31.fits |    4 ++--
             megacam/camera/ccd32.fits |    2 +-
             megacam/camera/ccd33.fits |    2 +-
             megacam/camera/ccd34.fits |    4 ++--
             megacam/camera/ccd35.fits |    2 +-
             36 files changed, 52 insertions(+), 52 deletions(-)
            

            Show
            price Paul Price added a comment - price@lsst-dev:~/LSST/obs/cfht (tickets/DM-5524) $ git sub commit e81cfda6a4e9c5ab382d91cfb47c9cfc4255649e Author: Paul Price <price@astro.princeton.edu> Date: Thu Mar 17 09:12:26 2016 -0500   fix megacam vertical overscan The vertical overscan was too large by a single pixel, for all CCDs. Edited via: path = "megacam/camera/ccd%02d.fits" for ii in range(36): ccd = afwTable.AmpInfoCatalog.readFits(path % ii) for amp in ccd: bbox = amp.getRawVerticalOverscanBBox() amp.setRawVerticalOverscanBBox(afwGeom.Box2I(bbox.getMin(), bbox.getDimensions() - afwGeom.Extent2I(1, 1))) ccd.writeFits(path % ii)   megacam/camera/ccd00.fits | 4 ++-- megacam/camera/ccd01.fits | 2 +- megacam/camera/ccd02.fits | 2 +- megacam/camera/ccd03.fits | 2 +- megacam/camera/ccd04.fits | 2 +- megacam/camera/ccd05.fits | 2 +- megacam/camera/ccd06.fits | 4 ++-- megacam/camera/ccd07.fits | 2 +- megacam/camera/ccd08.fits | 2 +- megacam/camera/ccd09.fits | 2 +- megacam/camera/ccd10.fits | 2 +- megacam/camera/ccd11.fits | 2 +- megacam/camera/ccd12.fits | 4 ++-- megacam/camera/ccd13.fits | 2 +- megacam/camera/ccd14.fits | 4 ++-- megacam/camera/ccd15.fits | 4 ++-- megacam/camera/ccd16.fits | 4 ++-- megacam/camera/ccd17.fits | 2 +- megacam/camera/ccd18.fits | 4 ++-- megacam/camera/ccd19.fits | 4 ++-- megacam/camera/ccd20.fits | 2 +- megacam/camera/ccd21.fits | 4 ++-- megacam/camera/ccd22.fits | 4 ++-- megacam/camera/ccd23.fits | 2 +- megacam/camera/ccd24.fits | 4 ++-- megacam/camera/ccd25.fits | 2 +- megacam/camera/ccd26.fits | 6 +++--- megacam/camera/ccd27.fits | 2 +- megacam/camera/ccd28.fits | 4 ++-- megacam/camera/ccd29.fits | 2 +- megacam/camera/ccd30.fits | 2 +- megacam/camera/ccd31.fits | 4 ++-- megacam/camera/ccd32.fits | 2 +- megacam/camera/ccd33.fits | 2 +- megacam/camera/ccd34.fits | 4 ++-- megacam/camera/ccd35.fits | 2 +- 36 files changed, 52 insertions(+), 52 deletions(-)
            Hide
            boutigny Dominique Boutigny added a comment -

            I have 2 comments :
            1- You subtracted 1 in both x and y direction, I think it should have been -1 in y only : see : http://www.cfht.hawaii.edu/Science/CFHTLS-DATA/rawdata.html
            2- It would be preferable to fix the geometry in camera.py / bin/genCameraRepository.py because next time someone will re-generate the Megacam geometry, they will peek up the wrong overscan dimension size again.
            Simon Krughoff may comment

            Show
            boutigny Dominique Boutigny added a comment - I have 2 comments : 1- You subtracted 1 in both x and y direction, I think it should have been -1 in y only : see : http://www.cfht.hawaii.edu/Science/CFHTLS-DATA/rawdata.html 2- It would be preferable to fix the geometry in camera.py / bin/genCameraRepository.py because next time someone will re-generate the Megacam geometry, they will peek up the wrong overscan dimension size again. Simon Krughoff may comment
            Hide
            price Paul Price added a comment -

            1. Removed the silly subtraction in x. Would you check again, please?
            2. There's nothing to fix in camera.py (all the amp definitions are in the FITS files), and I would like to avoid touching genCameraRepository.py because it's rather more complicated, and it was only used for transition so there should be no need to re-generate the geometry. In fact, I hope genCameraRepository.py and the cameraGeom PAF files will be removed now we're well after the transition, but that's up to Simon Krughoff.

            Show
            price Paul Price added a comment - 1. Removed the silly subtraction in x. Would you check again, please? 2. There's nothing to fix in camera.py (all the amp definitions are in the FITS files), and I would like to avoid touching genCameraRepository.py because it's rather more complicated, and it was only used for transition so there should be no need to re-generate the geometry. In fact, I hope genCameraRepository.py and the cameraGeom PAF files will be removed now we're well after the transition, but that's up to Simon Krughoff .
            Hide
            krughoff Simon Krughoff added a comment -

            I think I agree with Paul Price that we should get rid of the transition code. We can always get it back from the history if we really need to.

            Show
            krughoff Simon Krughoff added a comment - I think I agree with Paul Price that we should get rid of the transition code. We can always get it back from the history if we really need to.
            Hide
            boutigny Dominique Boutigny added a comment - - edited

            I checked that it works for the whole focal plane.
            For 2- Yes, I now remember that Simon Krughoff explained that to me

            It is ok to merge

            Show
            boutigny Dominique Boutigny added a comment - - edited I checked that it works for the whole focal plane. For 2- Yes, I now remember that Simon Krughoff explained that to me It is ok to merge
            Hide
            price Paul Price added a comment -

            Thanks, both.

            Merged to master.

            Show
            price Paul Price added a comment - Thanks, both. Merged to master.

              People

              Assignee:
              price Paul Price
              Reporter:
              price Paul Price
              Reviewers:
              Dominique Boutigny
              Watchers:
              Dominique Boutigny, Paul Price, Simon Krughoff
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: