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

Vertical overscan off by one again

    XMLWordPrintable

    Details

      Description

      In DM-5524 Paul Price fixed the vertical overscan by directly editing the amp info catalogs, but didn't mark the camera generating code as bad. In DM-6147 I regenerated the files, reintroducing the problem. The problem seems to be a subtle bug in the camera generating code. Rather than try to fix it, I'll convert the fixed catalogs directly and mark the generating code as broken. Paul Price will issue an RFC that suggests a better way to handle generating amp info and once that is dealt with we can come up with a more permanent fix (e.g. delete the generating code or fix it).

        Attachments

          Issue Links

          blocks

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-6057 v12.0 [Winter 2016 / Extra 2016] release

          • Done
          relates to

          RFC - An issue whose primary purpose is to advertise an intent and allow stakeholders to comment. RFC-192 Single source for camera data

          • Adopted

          Bug - A problem which impairs or prevents the functions of the product. DM-5524 MegaCam vertical overscan dimensions off by one

          • Done

          Improvement - An improvement or enhancement to an existing feature or task. DM-6147 Set SUSPECT mask in ISR task and make saturation a double

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            price Paul Price added a comment -

            Looking at a random CCD:

            pprice@tiger-sumire:~/LSST/obs/cfht (tickets/DM-6246=) $ python -c 'from lsst.afw.table import AmpInfoCatalog; ccd = AmpInfoCatalog.readFits("megacam/camera/ccd13.fits"); print [amp.getRawVerticalOverscanBBox() for amp in ccd]'
            		 
            [Box2I(Point2I(32, 4612), Extent2I(1023, 31)), Box2I(Point2I(1056, 4612), Extent2I(1023, 31))]

            I think these should be Extent2I(1024, 32). Dominique Boutigny says on DM-5524:

            They should be 1024x32...

            Why have all the comments been removed from the megacam/camera/camera.py file?

            Show
            price Paul Price added a comment - Looking at a random CCD: pprice@tiger-sumire:~/LSST/obs/cfht (tickets/DM-6246=) $ python -c 'from lsst.afw.table import AmpInfoCatalog; ccd = AmpInfoCatalog.readFits("megacam/camera/ccd13.fits"); print [amp.getRawVerticalOverscanBBox() for amp in ccd]' [Box2I(Point2I(32, 4612), Extent2I(1023, 31)), Box2I(Point2I(1056, 4612), Extent2I(1023, 31))] I think these should be Extent2I(1024, 32) . Dominique Boutigny says on DM-5524 : They should be 1024x32... Why have all the comments been removed from the megacam/camera/camera.py file?
            Hide
            Permalink
            rowen Russell Owen added a comment - - edited

            Regarding the vertical overscan dimensions: DM-5524 made them 31 high, but the ticket says they should be 32. Apparently the amp info tables have been working so I propose to use this ticket to restore what was done in DM-5524. If they need another change, I suggest that be done on another ticket. Dominique Boutigny are the current tables adequate?

            Regarding comments in camera.py: I think the camera generating code puts them in, but they appear to have been lost at some point. When I converted the amp info tables I just grabbed the camera.py from the same commit (60e4cb0 which was the last merge to master before DM-6147) and that version of camera.py was missing the comments. I agree they are nice to have, so I have reverted that change – camera.py once again has the comments that were restored in DM-6147.

            Show
            rowen Russell Owen added a comment - - edited Regarding the vertical overscan dimensions: DM-5524 made them 31 high, but the ticket says they should be 32. Apparently the amp info tables have been working so I propose to use this ticket to restore what was done in DM-5524 . If they need another change, I suggest that be done on another ticket. Dominique Boutigny are the current tables adequate? Regarding comments in camera.py : I think the camera generating code puts them in, but they appear to have been lost at some point. When I converted the amp info tables I just grabbed the camera.py from the same commit ( 60e4cb0 which was the last merge to master before DM-6147 ) and that version of camera.py was missing the comments. I agree they are nice to have, so I have reverted that change – camera.py once again has the comments that were restored in DM-6147 .
            Hide
            Permalink
            boutigny Dominique Boutigny added a comment -

            Russell Owen I confirm that the vertical overscan dimensions which has been implemented in DM-5524 was adequate. The description of the Megacam CCDs is here : http://www.cfht.hawaii.edu/Science/CFHTLS-DATA/rawdata.html

            Show
            boutigny Dominique Boutigny added a comment - Russell Owen I confirm that the vertical overscan dimensions which has been implemented in DM-5524 was adequate. The description of the Megacam CCDs is here : http://www.cfht.hawaii.edu/Science/CFHTLS-DATA/rawdata.html

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Paul Price
              Watchers:
              Dominique Boutigny, Frossie Economou, Paul Price, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.