Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-605

Add camera parity to Camera geometry

    Details

    • Type: RFC
    • Status: Adopted
    • Resolution: Unresolved
    • Component/s: DM

      Description

      In order to get the CFHT MegaPrime and DECam camera orientations correct on the sky when creating an initial SkyWcs from the telescope boresight and camera geometry (see DM-20201 and DM-20188), we need to flip DECam along X. This parity flip should be encoded in the camera geometry itself, so that we do not need to specify anything special in each obs package when loading the raw images.

      Russell Owen and I have discussed a few options. Here, I propose that we incorporate this parity flip as part of the Transform between the FOCAL_PLANE and FIELD_ANGLE coordinate systems. This would change the definition of the FIELD_ANGLE in CameraSys.h such that it may not always be true that "The orientation of the x,y axes is the same as FOCAL_PLANE." A benefit would be that FIELD_ANGLE will always have a consistent relationship to SKY for all cameras, being a pure rotation and projection to the sphere. Incorporating the parity flip here means that the RotType=SKY definition in VisitInfo would have a single meaning; it currently says "+Y is along N and +X is along E/W depending on handedness."

      We would add a getFocalPlaneParity() read-only method to Camera so that the user can determine whether there is such a parity flip. This value would be retrieved from the Transform between FOCAL_PLANE and FIELD_ANGLE.

      A challenge of adding this transform is that we do not currently have a coherent system for describing camera geometries. Most existing obs packages have either a camGeom/ or camera/ directory with a python config and a bunch of FITS files (built on pex_config) while obs_lsst uses an YAML format (which also isn't documented, see e.g. DM-12682 and DM-17532).

      Implementation detail: A possible solution to how to deal with existing pex_config Cameras is to add a focalPlaneParity boolean to the pex_config camera definitions that gets "automatically" pulled into the appropriate transform when the Camera is created. YAML cameras would have the parity incorporated as part of the transformation directly.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            Why can't we change the lower-level specification of the DECam CCDs so that we don't need to make this flip?

            Aside from being more disruptive than the proposal on the table, I don't think we can do that and maintain two more important properties of all current cameras:

            • the PIXEL coordinate system should correspond to the detector electronics;
            • there should not be a flip between PIXEL and FOCAL_PLANE.

            To clarify, my understanding is that this is not "another" flip; this is the one flip that is necessary to make the geometry correct for DECam (and presumably at least a handful of other instruments).

            DMTN on this would be very useful indicating which transforms belong in which packages – perhaps Tim Jenness and John Parejko you could draft such a document. Also indicate backward comparability of the proposal.

            We have some documentation on what coordinate systems should exist and what invariants they should have in the code documentation at https://pipelines.lsst.io/modules/lsst.afw.cameraGeom/cameraGeom.html#camera-coordinate-systems. I think it's clearly in scope for the implementation of this RFC (but hopefully not a blocker on its acceptance) to expand that to document where any parity flips should go, and I'd be in favor of expanding it in other ways as well (a diagram would be nice), as it's a bit sparse at the moment. All obs* packages are responsible for defining all of these coordinate systems and the transforms between them.

             

            In order to get this RFC moving again, I think we should ignore both my earlier comments about a new standard coordinate system between FOCAL_PLANE and FIELD_ANGLE and the last two paragraphs of the original RFC description about consistent camera definitions: both of those are distracting scope creep for this RFC, which is really about fixing a bug in our current camera geometry for DECam by defining what our convention should be for parity flips. I think what's proposed here is both the least disruptive change and most sane convention to adopt, and we should discuss additional coordinate systems or better camera definition consistency/documentation in other venues (note that we're in the early stages of addressing the latter on DM-18610; not all of that work is ticketed, but moving everything to YAML definitions and documenting that better is definitely in the works).

             

            Show
            jbosch Jim Bosch added a comment - Why can't we change the lower-level specification of the DECam CCDs so that we don't need to make this flip? Aside from being more disruptive than the proposal on the table, I don't think we can do that and maintain two more important properties of all current cameras: the PIXEL coordinate system should correspond to the detector electronics; there should not be a flip between PIXEL and FOCAL_PLANE. To clarify, my understanding is that this is not "another" flip; this is the one flip that is necessary to make the geometry correct for DECam (and presumably at least a handful of other instruments). DMTN on this would be very useful indicating which transforms belong in which packages – perhaps Tim Jenness and John Parejko you could draft such a document. Also indicate backward comparability of the proposal. We have some documentation on what coordinate systems should exist and what invariants they should have in the code documentation at https://pipelines.lsst.io/modules/lsst.afw.cameraGeom/cameraGeom.html#camera-coordinate-systems . I think it's clearly in scope for the implementation of this RFC (but hopefully not a blocker on its acceptance) to expand that to document where any parity flips should go, and I'd be in favor of expanding it in other ways as well (a diagram would be nice), as it's a bit sparse at the moment. All obs* packages are responsible for defining all of these coordinate systems and the transforms between them.   In order to get this RFC moving again, I think we should ignore both my earlier comments about a new standard coordinate system between FOCAL_PLANE and FIELD_ANGLE and the last two paragraphs of the original RFC description about consistent camera definitions: both of those are distracting scope creep for this RFC, which is really about fixing a bug in our current camera geometry for DECam by defining what our convention should be for parity flips. I think what's proposed here is both the least disruptive change and most sane convention to adopt, and we should discuss additional coordinate systems or better camera definition consistency/documentation in other venues (note that we're in the early stages of addressing the latter on DM-18610 ; not all of that work is ticketed, but moving everything to YAML definitions and documenting that better is definitely in the works).  
            Hide
            jbosch Jim Bosch added a comment -

            I talked with Robert Lupton today directly about this RFC.  The TL;DR is that the points below do not change the big picture of whether this RFC should be adopted (we're both on board), but it does change some of the reasoning:

            • We think that it's not possible for the optics to introduce a parity flip for DECam and not other prime-focus cameras like HSC without the parity conventions differing somewhere else - only mirrors can change parity, not lenses.  That's presumably just a question of whether the focal plane is defined "looking down" or "looking out".
            • My statement that there are no current parity flips between PIXELS and FOCAL_PLANE was true, but irrelevant - individual amplifiers within the same chip can have parity flips.  That doesn't affect the PIXELS coordinate system (which defines post-ISR pixel coordinates), but it does mean that my previous assertion that there is some kind of physical connection between PIXELS and FOCAL_PLANE that impacts parity discussions was totally bogus.

            The fact that there is a parity flip between PIXELS and sky for DECam but not other prime-focus cameras is thus just a convention difference, and it's a reasonable question whether we should address that (as Robert Lupton originally proposed) by changing our DECam definition to adopt the convention used elsewhere (essentially by flipping the parity flag on all of its amplifiers, and updating their bounding boxes accordingly).  But there are three good reasons not to do this, and to instead let DECam be different:

            • changing the DECam amplifier geometry is a bit harder and more disruptive than the RFC proposal;
            • there may be other cameras we want to support in the future that do have a genuine optical parity flip (because they have an even number of mirrors) even with the same convention for whether the focal plane is defined as "looking down" or "looking out".
            • external DECam documents use this convention (I think John Parejko has investigated this and may be able to point to examples), and consistency with those is desirable.

            However, I think this does highlight that obs package documentation should clearly state whether the focal plane is defined "looking down" or "looking out", and I think that's reasonably in scope for the implementation of this RFC.

             

            Show
            jbosch Jim Bosch added a comment - I talked with Robert Lupton today directly about this RFC.  The TL;DR is that the points below do not change the big picture of whether this RFC should be adopted (we're both on board), but it does change some of the reasoning: We think that it's not possible for the optics to introduce a parity flip for DECam and not other prime-focus cameras like HSC without the parity conventions differing somewhere else - only mirrors can change parity, not lenses.  That's presumably just a question of whether the focal plane is defined "looking down" or "looking out". My statement that there are no current parity flips between PIXELS and FOCAL_PLANE was true, but irrelevant - individual amplifiers within the same chip can have parity flips.  That doesn't affect the PIXELS coordinate system (which defines post-ISR pixel coordinates), but it does mean that my previous assertion that there is some kind of physical connection between PIXELS and FOCAL_PLANE that impacts parity discussions was totally bogus. The fact that there is a parity flip between PIXELS and sky for DECam but not other prime-focus cameras is thus just a convention difference, and it's a reasonable question whether we should address that (as Robert Lupton originally proposed) by changing our DECam definition to adopt the convention used elsewhere (essentially by flipping the parity flag on all of its amplifiers, and updating their bounding boxes accordingly).  But there are three good reasons not to do this, and to instead let DECam be different: changing the DECam amplifier geometry is a bit harder and more disruptive than the RFC proposal; there may be other cameras we want to support in the future that do have a genuine optical parity flip (because they have an even number of mirrors) even with the same convention for whether the focal plane is defined as "looking down" or "looking out". external DECam documents use this convention (I think John Parejko has investigated this and may be able to point to examples), and consistency with those is desirable. However, I think this does highlight that obs package documentation should clearly state whether the focal plane is defined "looking down" or "looking out", and I think that's reasonably in scope for the implementation of this RFC.  
            Hide
            Parejkoj John Parejko added a comment -

            external DECam documents use this convention (I think John Parejko has investigated this and may be able to point to examples), and consistency with those is desirable.

            The two diagrams on this page show exactly the situation that resulted in this RFC:

            http://www.ctio.noao.edu/noao/node/2250

            Show
            Parejkoj John Parejko added a comment - external DECam documents use this convention (I think John Parejko has investigated this and may be able to point to examples), and consistency with those is desirable. The two diagrams on this page show exactly the situation that resulted in this RFC: http://www.ctio.noao.edu/noao/node/2250
            Hide
            tjenness Tim Jenness added a comment -

            DMCCB have no further comment on this RFC so long as a triggered ticket is created to cover the task of updating the camera geometry documentation.

            Show
            tjenness Tim Jenness added a comment - DMCCB have no further comment on this RFC so long as a triggered ticket is created to cover the task of updating the camera geometry documentation.
            Hide
            Parejkoj John Parejko added a comment -

            DM-20746 is the implementing ticket, including updating the cameraGeom docs.

            Show
            Parejkoj John Parejko added a comment - DM-20746 is the implementing ticket, including updating the cameraGeom docs.

              People

              • Assignee:
                Parejkoj John Parejko
                Reporter:
                Parejkoj John Parejko
                Watchers:
                Christopher Waters, Jim Bosch, John Parejko, John Swinbank, Joshua Meyers, Paul Price, Robert Lupton, Russell Owen, Tim Jenness, Wil O'Mullane, Yusra AlSayyad
              • Votes:
                0 Vote for this issue
                Watchers:
                11 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Planned End:

                  Summary Panel