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

Correct for distortion in matchOptimisticB astrometry matcher

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_astrom, obs_subaru
    • Labels:
      None

      Description

      matchOptimisticB does not correct for distortion, although an estimate of the distortion is available. We suspect that doing the matching on the celestial sphere might be ideal, but matching on a tangent plane has worked for HSC.

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            I propose to take Russell Owen's comments as the review. Is this OK with you, Simon Krughoff?

            Show
            price Paul Price added a comment - I propose to take Russell Owen 's comments as the review. Is this OK with you, Simon Krughoff ?
            Hide
            price Paul Price added a comment -

            I will update the matchOptimisticB API to require a Wcs.

            In _fitWcs, the wcs is updated for the next iteration (note that the call is in a loop).

            I'll modify the code to use Angle instead of implicitly assuming degrees.

            Show
            price Paul Price added a comment - I will update the matchOptimisticB API to require a Wcs . In _fitWcs , the wcs is updated for the next iteration (note that the call is in a loop). I'll modify the code to use Angle instead of implicitly assuming degrees.
            Hide
            price Paul Price added a comment -

            I've pushed two fixup commits for review:

            commit 6f741decada4b41983ba25d1ca17a12c47231897
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Aug 26 12:55:31 2015 -0400
             
                fixup! matchOptimisticB: correct for distortion
             
             include/lsst/meas/astrom/matchOptimisticB.h |  8 +++----
             src/matchOptimisticB.cc                     | 36 +++++++++--------------------
             2 files changed, 15 insertions(+), 29 deletions(-)
             
            commit c9ac6fecbe4d86ab9b8f305dfe2d62089b66fd4f
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Aug 26 12:55:50 2015 -0400
             
                fixup! matchOptimisticB: fix use of allowedNonperpDeg
             
             src/matchOptimisticB.cc | 9 +++++----
             1 file changed, 5 insertions(+), 4 deletions(-)
            

            Show
            price Paul Price added a comment - I've pushed two fixup commits for review: commit 6f741decada4b41983ba25d1ca17a12c47231897 Author: Paul Price <price@astro.princeton.edu> Date: Wed Aug 26 12:55:31 2015 -0400   fixup! matchOptimisticB: correct for distortion   include/lsst/meas/astrom/matchOptimisticB.h | 8 +++---- src/matchOptimisticB.cc | 36 +++++++++-------------------- 2 files changed, 15 insertions(+), 29 deletions(-)   commit c9ac6fecbe4d86ab9b8f305dfe2d62089b66fd4f Author: Paul Price <price@astro.princeton.edu> Date: Wed Aug 26 12:55:50 2015 -0400   fixup! matchOptimisticB: fix use of allowedNonperpDeg   src/matchOptimisticB.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
            Hide
            rowen Russell Owen added a comment -

            Many thanks for the enhancements. Changing theta to an Angle was a good idea. I suspect other variables would benefit from the same treatment, but suggest that be left for a future cleanup pass (maybe one that also removes RecordProxy from the public namespace).

            One remaining request: please update the documentation of matchOptimisticB in matchOptimisticB.h. Presently it says that posRefCat reads "centroid" and "hasCentroid" fields, but that is no longer the case, thanks to your improvements. If you are willing, you could also add that the sourceCat fields read are "Centroid_x", "Centroid_y" and control.sourceFluxField.To aid that I provide the following suggested text, which I verified has a nicer format in HTML (significantly more so than the old text):

                @param[in] posRefCat  catalog of position reference stars; fields read:
                    - "coord"
                    - control.refFluxField
                @param[in] sourceCat  catalog of detected sources; fields read:
                    - "Centroid_x"
                    - "Centroid_y"
                    - control.refFluxField
            

            I will also note a quirk of the Doxygen that I should have noticed before. I have no idea how to fix it and am not proposing you try. I'm just recording it. The documentation for the function "matchOptimisticB" is hard to find. The link of that name under the tab "Namespaces: Namespace List" points to documentation of the python module of that name, not the C++ function. I found documentation for the C++ function under "Files:matchOptimisticB.h". Also also found it under the tab "Namespaces: Namespace Members", but that page is really cluttered and clumsy to use and the linked entry includes an unwanted entry for the function from the .cc file, even though the .cc file contains no doxygen comments.

            Show
            rowen Russell Owen added a comment - Many thanks for the enhancements. Changing theta to an Angle was a good idea. I suspect other variables would benefit from the same treatment, but suggest that be left for a future cleanup pass (maybe one that also removes RecordProxy from the public namespace). One remaining request: please update the documentation of matchOptimisticB in matchOptimisticB.h. Presently it says that posRefCat reads "centroid" and "hasCentroid" fields, but that is no longer the case, thanks to your improvements. If you are willing, you could also add that the sourceCat fields read are "Centroid_x", "Centroid_y" and control.sourceFluxField.To aid that I provide the following suggested text, which I verified has a nicer format in HTML (significantly more so than the old text): @param[in] posRefCat catalog of position reference stars; fields read: - "coord" - control.refFluxField @param[in] sourceCat catalog of detected sources; fields read: - "Centroid_x" - "Centroid_y" - control.refFluxField I will also note a quirk of the Doxygen that I should have noticed before. I have no idea how to fix it and am not proposing you try. I'm just recording it. The documentation for the function "matchOptimisticB" is hard to find. The link of that name under the tab "Namespaces: Namespace List" points to documentation of the python module of that name, not the C++ function. I found documentation for the C++ function under "Files:matchOptimisticB.h". Also also found it under the tab "Namespaces: Namespace Members", but that page is really cluttered and clumsy to use and the linked entry includes an unwanted entry for the function from the .cc file, even though the .cc file contains no doxygen comments.
            Hide
            price Paul Price added a comment -

            Thanks Russell. I updated the doc, rebased and merged to master.

            Show
            price Paul Price added a comment - Thanks Russell. I updated the doc, rebased and merged to master.

              People

              • Assignee:
                price Paul Price
                Reporter:
                price Paul Price
                Reviewers:
                Russell Owen
                Watchers:
                Jim Bosch, Paul Price, Russell Owen, Simon Krughoff
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel