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

Move AstrometryTask source selection from "matcher" into AstrometryTask

    XMLWordPrintable

Details

    • Bug
    • Status: Done
    • Resolution: Done
    • None
    • None
    • None
    • 2
    • DRP S19-4, DRP S19-5
    • Data Release Production

    Description

      It appears that unless config.forceKnownWcs=True for the AstrometryTask (called in processCcd), then the referenceSelection task is not actually run.
      These selectors are called only in loadAndMatch (https://github.com/lsst/meas_astrom/blob/b71a5648e2899270502e19d7bb47e8521766682d/python/lsst/meas/astrom/ref_match.py#L91) and not in solve (https://github.com/lsst/meas_astrom/blob/b71a5648e2899270502e19d7bb47e8521766682d/python/lsst/meas/astrom/astrometry.py#L145), and loadAndMatch is only called if forceKnownWcs is True: https://github.com/lsst/meas_astrom/blob/b71a5648e2899270502e19d7bb47e8521766682d/python/lsst/meas/astrom/astrometry.py#L137

      Further investigation has led to the realization that there are two source selectors that may be called in certain circumstances due to duplication of configs.

      This investigation led to RFC-589, and this is now the implementation ticket for that RFC:

      • The sourceSelector be removed from the matcher tasks (namely, MatchOptimisticBTask and MatchPessimisticBTask)
      • The sourceSelection in AstrometryTask be changed to a SourceSelectorRegistry.
      • The source selection and reference selection will be called by AstrometryTask prior to the match-and-fit iterations. This will also make things (marginally) faster because in the current usage the (identical) source selection is repeated for each iteration.

      Attachments

        Issue Links

          Activity

            erykoff Eli Rykoff added a comment -

            Ready to be reviewed properly with all the outlined fixes added.

            erykoff Eli Rykoff added a comment - Ready to be reviewed properly with all the outlined fixes added.

            Do you need the pull requests in the validation data? I believe these configs are from the processed data not a config that is run with the dataset. I believe those live in the base config directory.

            cmorrison Chris Morrison [X] (Inactive) added a comment - Do you need the pull requests in the validation data? I believe these configs are from the processed data not a config that is run with the dataset. I believe those live in the base config directory.
            erykoff Eli Rykoff added a comment -

            Okay, that wasn't clear to me. I'm happy to kill those PRs.

            erykoff Eli Rykoff added a comment - Okay, that wasn't clear to me. I'm happy to kill those PRs.

            Have you run this through any of the validate_drp scripts and looked at if any of the asymmetry or similar metrics change relative to what's in squash or chronograph?

            cmorrison Chris Morrison [X] (Inactive) added a comment - Have you run this through any of the validate_drp scripts and looked at if any of the asymmetry or similar metrics change relative to what's in squash or chronograph?
            erykoff Eli Rykoff added a comment -

            I have not run through validate_drp; this is not something I have any experience with. However, I believe the behavior should be the same as before in terms of the matching. Also, I did run through ci_hsc just in case and it didn't log any problems.

            erykoff Eli Rykoff added a comment - I have not run through validate_drp ; this is not something I have any experience with. However, I believe the behavior should be the same as before in terms of the matching. Also, I did run through ci_hsc just in case and it didn't log any problems.

            Thanks, Eli. Code looks fine. Assuming that Jenkins with ci_hsc is passing (could you paste a link to the Jenkins job in the comments, please) and you rebase squash some of the "fix typo" commits on meas_astrom, go ahead a merge.

            cmorrison Chris Morrison [X] (Inactive) added a comment - Thanks, Eli. Code looks fine. Assuming that Jenkins with ci_hsc is passing (could you paste a link to the Jenkins job in the comments, please) and you rebase squash some of the "fix typo" commits on meas_astrom, go ahead a merge.
            erykoff Eli Rykoff added a comment -

            Jenkins on ci_hsc here: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29656/pipeline
            Will rebase and squash, and one last jenkins to make extra sure before I merge.

            erykoff Eli Rykoff added a comment - Jenkins on ci_hsc here: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/29656/pipeline Will rebase and squash, and one last jenkins to make extra sure before I merge.

            People

              erykoff Eli Rykoff
              erykoff Eli Rykoff
              Chris Morrison [X] (Inactive)
              Chris Morrison [X] (Inactive), Eli Rykoff
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.