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

Make TAN_PIXELS cameraGeom coordinate be with respect to the center of the focal plane

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      The TAN_PIXELS cameraGeom coordinate system is presently defined to match PIXELS at the center of each CCD. This was a mistake on my part and I'd like to correct it. TAN_PIXELS was intended to model the effects of optical distortion, and to do so in a straightforward way TAN_PIXELS should match PIXELS at the optical axis (origin of the PUPIL frame).

      Furthermore, TAN_PIXELS does not presently support rectangular pixels correctly (technically this is a misfeature, not a bug; it uses a mean pixel scale, and is documented as such). I propose to fix that, as well. With the proposed new definition of TAN_PIXELS the fix is trivial. Note that we do not yet have any cameras with rectangular pixels, so fixing this will not make any difference to current cameras.

      One of the primary uses I intended for TAN_PIXELS was to provide a proper WCS given an initial TAN WCS. However, this is presently broken (DM-6529) because of the incorrect definition of TAN_PIXELS. There are other ways to fix DM-6529, but changing the definition of TAN_PIXELS as proposed is a clean, simple solution, and the results will be easier to understand.

      An implementation is available on afw tickets/DM-2800 in case you want to try it out to see what difference it makes.

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            I support this change, but I think we should insist that both the default matcher and astrometry.net be demonstrated as working successfully before merging.

            Show
            price Paul Price added a comment - I support this change, but I think we should insist that both the default matcher and astrometry.net be demonstrated as working successfully before merging.
            Hide
            Parejkoj John Parejko added a comment -

            I also support this change.

            Show
            Parejkoj John Parejko added a comment - I also support this change.
            Hide
            jbosch Jim Bosch added a comment -

            This sounds entirely reasonable. Just so I have a sense for what it might impact, do you know of any places where we use TAN_PIXELS other than "flattening" source coordinates to match them to the reference catalog?

            Show
            jbosch Jim Bosch added a comment - This sounds entirely reasonable. Just so I have a sense for what it might impact, do you know of any places where we use TAN_PIXELS other than "flattening" source coordinates to match them to the reference catalog?
            Hide
            rowen Russell Owen added a comment - - edited

            Paul Price: I agree that testing by running our code on real data is important, and your point about testing the a.net solver is well taken. I have done that with validate_drp using afw tickets/DM-2280 and it all works as expected:

            • for DECam and CFHT the results are identical for CCDs near the center
            • for CFHT the results are better with the new code for CCD0 (upper left corner) than the old code with AstrometryTask and unchanged with the a.net astrometry solver

            In addition, Bob Armstrong tested afw tickets/DM-2280 and reports that it fixes DM-6923 for HSC.

            Jim Bosch I looked through the code. TAN_PIXELS is used (if available) by the second moment star selector to refine Ixx, etc.; it appears a second order effect so I don't expect this to change it significantly, but that star selector has no unit tests and is not used, so I can't easily prove that. TAN_PIXELS is also used to display the effects of distortion. I hope this change will generally be an improvement there, but I suppose it depends on exactly what you were trying to display.

            Show
            rowen Russell Owen added a comment - - edited Paul Price : I agree that testing by running our code on real data is important, and your point about testing the a.net solver is well taken. I have done that with validate_drp using afw tickets/ DM-2280 and it all works as expected: for DECam and CFHT the results are identical for CCDs near the center for CFHT the results are better with the new code for CCD0 (upper left corner) than the old code with AstrometryTask and unchanged with the a.net astrometry solver In addition, Bob Armstrong tested afw tickets/ DM-2280 and reports that it fixes DM-6923 for HSC. Jim Bosch I looked through the code. TAN_PIXELS is used (if available) by the second moment star selector to refine Ixx, etc.; it appears a second order effect so I don't expect this to change it significantly, but that star selector has no unit tests and is not used, so I can't easily prove that. TAN_PIXELS is also used to display the effects of distortion. I hope this change will generally be an improvement there, but I suppose it depends on exactly what you were trying to display.
            Hide
            rowen Russell Owen added a comment -

            Adopted as specified

            Show
            rowen Russell Owen added a comment - Adopted as specified

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                rowen Russell Owen
                Watchers:
                Bob Armstrong, Jim Bosch, John Parejko, John Swinbank, Paul Price, Robert Lupton, Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Planned End:

                  Summary Panel