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

Make astrometry distortion model configurable

    Details

      Description

      Jointcal has two distortion models implemented: SimplePolyModel and ConstrainedPolyModel. Until now, I've been using the former because it was the default when I started using jointcal. The choice of model should be a config parameter (they share an API), which will require bringing the Constrained model up to match other changes in the C++ code.

      The CFHT or DECam files in testdata_jointcal should work as test datasets: Constrained wants a full mosaic. To fully exercise it, it would be best to use the latest validation_data_hsc for multiple full-focal plane visits.

      Note also this note from Pierre Astier about getting the Constrained model to work:

      The problem with ConstrainedPolyModel is that
      at line 72, a test reads :

      if ((chipp == _chipMap.end()) && visit == refVisit )

      which should just be:
      if (chipp == _chipMap.end())

      because the initilization should be done anyway, possibly with a warning
      if the chip is found on a different visit. Maybe the initializer should be
      im.Pix2CommonTangentPlane() rather than im.Pix2TangentPlane()
      but it is not mandatory.

      For this model, the first fitting steps we are using read:
      astromFit.Minimize("DistortionsVisit"); // those are not initialized at all
      astromFit.Minimize("Positions");
      astromFit.Minimize("DistortionsChip");

        Attachments

          Issue Links

            Activity

            Hide
            fred3m Fred Moolekamp added a comment -

            Looks good to me. Sorry if you were waiting for a reply before closing this ticket out, but as long as the changes pass Jenkins tests I think this is fine to close.

            Show
            fred3m Fred Moolekamp added a comment - Looks good to me. Sorry if you were waiting for a reply before closing this ticket out, but as long as the changes pass Jenkins tests I think this is fine to close.
            Hide
            Parejkoj John Parejko added a comment -

            Thanks for your note. The reason I haven't merged is that my post-review refactor seems to have brought up a Heisenbug in the C++ (and only on macOS clang, so Jenkins passes!), which I'm now trying to track down.

            Show
            Parejkoj John Parejko added a comment - Thanks for your note. The reason I haven't merged is that my post-review refactor seems to have brought up a Heisenbug in the C++ (and only on macOS clang, so Jenkins passes!), which I'm now trying to track down.
            Hide
            Parejkoj John Parejko added a comment - - edited
            Show
            Parejkoj John Parejko added a comment - - edited I pushed some not-insignificant changes in the process of fixing the segfaulting on macOS. Would you mind re-reviewing? The commits that matter are these: https://github.com/lsst/jointcal/pull/30/commits/03bb56b04fddadaaef5247485d59afe97dda7717 https://github.com/lsst/jointcal/pull/30/commits/170ecef397b565f3e7c3c4ab55088b129e5b6d3b https://github.com/lsst/jointcal/pull/30/commits/2e967286d41716179253365023540b9809f28b5c Thanks!
            Hide
            fred3m Fred Moolekamp added a comment -

            Everything looks good.

            Show
            fred3m Fred Moolekamp added a comment - Everything looks good.
            Hide
            Parejkoj John Parejko added a comment -

            Thanks for the quick turn-around on the re-review. I gave up trying to rebase flatten things beyond a few superficial ones, as the cleanup commits were mixed in with the other commits, which made it too much of a pain.

            Merged and done.

            Show
            Parejkoj John Parejko added a comment - Thanks for the quick turn-around on the re-review. I gave up trying to rebase flatten things beyond a few superficial ones, as the cleanup commits were mixed in with the other commits, which made it too much of a pain. Merged and done.

              People

              • Assignee:
                Parejkoj John Parejko
                Reporter:
                Parejkoj John Parejko
                Reviewers:
                Fred Moolekamp
                Watchers:
                Dominique Boutigny, Fred Moolekamp, John Parejko, Pierre Astier, Simon Krughoff
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel