# Vertical overscan off by one again

XMLWordPrintable

## Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
0.5
• Sprint:
Alert Production X16 - 5
• Team:

## 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).

## Activity

Hide
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 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
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
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
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
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:
Russell Owen
Reporter:
Russell Owen
Reviewers:
Paul Price
Watchers:
Dominique Boutigny, Frossie Economou, Paul Price, Russell Owen
0 Vote for this issue
Watchers:
4 Start watching this issue

## Dates

• Created:
Updated:
Resolved: