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

The TAN-SIP WCS fitter does a poor job with significant distortion and for high order SIP

    Details

    • Type: Bug
    • Status: Won't Fix
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_astrom
    • Labels:
      None
    • Team:
      Alert Production

      Description

      The tan-sip fitter in meas_astrom (lsst.meas.astrom.sip.makeCreateWcsWithSip) seems to fit poorly with significant distortion and for high order SIP.

      Perhaps the simplest way to see this is to modify the existing test testCreateWcsWithSip.py to use higher orders and/or more distortion.

      However, I have also attached sample code (partly based on that test) that shows the problem in more detail. The points it fits are uniformly distributed over a grid in the effective bbox, and so can be as dense as desired in order to provide enough information to fit to high order.

      Notes:

      • The sample code requires the current master of afw, since it uses DistortedTanWcs.
      • I observe that fitting a pure TAN WCS succeeds with order 3 and 4, but fails with order 5
      • I observe that fitting an offset radial distortion fails mildly at order 3 and spectacularly at orders 5 and 6
      • The code continues even after a test fails, in order to show how badly the higher-order SIP polynomials fail. Hence it claims to succeed at the end, even though tests failed.
      • The sample code uses crpix=15000, 4000 to emulate a CCD that is far from the center of the field. The fit bbox is centered on that crpix. Nonetheless, one issue may be that TAN-SIP WCS (or at least our fitter) cannot handle such a large crpix.

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            How does the fitter from hscAstrom fare?

            Show
            price Paul Price added a comment - How does the fitter from hscAstrom fare?
            Hide
            rowen Russell Owen added a comment - - edited

            From what I can tell the hscAstrom fitters (one for the TAN terms, the other for the SIP terms) fare much worse. They had no unit tests, so I adapted testCreateWcsWithSip from meas_astrom into a pair of tests (one for each fitter) and I could not get either fitter to pass. I pushed a fork of hscAstrom https://github.com/r-owen/hscAstrom that has the tests (and attempted to issue a pull request, but github rejected with a message that suggested a github bug).

            Clearly we need to straighten this out and come up with a good, robust TAN-SIP fitter. if there are intrinsic limitations that these tests are exceeding, then perhaps we need better documentation of those limitations.

            Show
            rowen Russell Owen added a comment - - edited From what I can tell the hscAstrom fitters (one for the TAN terms, the other for the SIP terms) fare much worse. They had no unit tests, so I adapted testCreateWcsWithSip from meas_astrom into a pair of tests (one for each fitter) and I could not get either fitter to pass. I pushed a fork of hscAstrom https://github.com/r-owen/hscAstrom that has the tests (and attempted to issue a pull request, but github rejected with a message that suggested a github bug). Clearly we need to straighten this out and come up with a good, robust TAN-SIP fitter. if there are intrinsic limitations that these tests are exceeding, then perhaps we need better documentation of those limitations.
            Hide
            jbosch Jim Bosch added a comment -

            Paul Price reports on HSC-1163 that in fact we've been using the meas_astrom SIP fitter on the HSC side for a long time now (with the hscAstrom matcher), so the rumor that the hscAstrom fitter is more battle-tested is completely unfounded, and we should be focusing our attention on improving the meas_astrom fitter or replacing it from scratch. Naoki Yasuda is working on getting those unit tests passing on hscAstrom in HSC-1163, but at this point I don't think we have any reason to believe that will produce a fitter that's any better at problematic cases than the one in meas_astrom.

            Show
            jbosch Jim Bosch added a comment - Paul Price reports on HSC-1163 that in fact we've been using the meas_astrom SIP fitter on the HSC side for a long time now (with the hscAstrom matcher), so the rumor that the hscAstrom fitter is more battle-tested is completely unfounded, and we should be focusing our attention on improving the meas_astrom fitter or replacing it from scratch. Naoki Yasuda is working on getting those unit tests passing on hscAstrom in HSC-1163, but at this point I don't think we have any reason to believe that will produce a fitter that's any better at problematic cases than the one in meas_astrom.
            Hide
            rowen Russell Owen added a comment -

            We also need a TAN fitter, such as the HSC fitter – presently we rely on astrometry.net for that purpose, but HSC has its own and we want to emulate that. HSC-1163 includes a unit test for the TAN fitter that fails, so we still hope HSC gets to the bottom of that.

            Also note that our astrometry.net solver calls our TAN-SIP fitter iteratively, and only after producing what is likely a reasonable TAN solution.

            Show
            rowen Russell Owen added a comment - We also need a TAN fitter, such as the HSC fitter – presently we rely on astrometry.net for that purpose, but HSC has its own and we want to emulate that. HSC-1163 includes a unit test for the TAN fitter that fails, so we still hope HSC gets to the bottom of that. Also note that our astrometry.net solver calls our TAN-SIP fitter iteratively, and only after producing what is likely a reasonable TAN solution.
            Hide
            price Paul Price added a comment -

            I don't like the iteration on calling the TAN-SIP fitter, for two reasons:
            1. I think it was originally implemented to work around the distortion problems, which we've solved in a better way.
            2. The criterion for ending the iteration is when the raw number of matches decreases, which I don't think is very robust.

            I suggest that if we choose to iterate, we should simply iterate a preselected (and configurable) number of times.

            Show
            price Paul Price added a comment - I don't like the iteration on calling the TAN-SIP fitter, for two reasons: 1. I think it was originally implemented to work around the distortion problems, which we've solved in a better way. 2. The criterion for ending the iteration is when the raw number of matches decreases, which I don't think is very robust. I suggest that if we choose to iterate, we should simply iterate a preselected (and configurable) number of times.
            Hide
            rowen Russell Owen added a comment - - edited

            Paul Price: Dominique Boutigny found that one reason iteration is required for the TAN-SIP WCS fitter is that it fits first X and then Y, rather than both at once. Thus several iterations are required to converge. See DM-2719. The astrometry task iterates matching+fitting, but it would probably suffice to iterate just the fitter. I strongly suspect the TanWcsFitTask should iterate internally, but that's for another ticket.

            Show
            rowen Russell Owen added a comment - - edited Paul Price: Dominique Boutigny found that one reason iteration is required for the TAN-SIP WCS fitter is that it fits first X and then Y, rather than both at once. Thus several iterations are required to converge. See DM-2719 . The astrometry task iterates matching+fitting, but it would probably suffice to iterate just the fitter. I strongly suspect the TanWcsFitTask should iterate internally, but that's for another ticket.
            Hide
            rowen Russell Owen added a comment -

            Since this is a duplicate of DM-2719, which Dominque Boutigny fixed, I have marked this as "won't fix".

            Show
            rowen Russell Owen added a comment - Since this is a duplicate of DM-2719 , which Dominque Boutigny fixed, I have marked this as "won't fix".

              People

              • Assignee:
                rhl Robert Lupton
                Reporter:
                rowen Russell Owen
                Watchers:
                Dominique Boutigny, Jim Bosch, John Swinbank, Mario Juric, Paul Price, Robert Lupton, Russell Owen, Simon Krughoff
              • Votes:
                0 Vote for this issue
                Watchers:
                8 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel