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

matcherSourceSelector incorrectly uses nChild and footprints in isMultiple test.

    Details

    • Story Points:
      1
    • Epic Link:
    • Sprint:
      Alert Production S17 - 1, Alert Production S17 - 2
    • Team:
      Alert Production

      Description

      This bug was found by dm-square as a drop in match rms quality. The replacement for the SourceInfo class in matchOptimisticB.py has a test for isMultiple but this test was not used in the subsequent isGood or isUsable tests. The current implementation of the new matcherSourceSelector incorrectly uses this test and to parrot the performance of SourceInfo (which was the goal of DM-6824) this test should be removed.

        Attachments

          Issue Links

            Activity

            Hide
            cmorrison Chris Morrison added a comment -

            Removed from isMultiple test from matcherSourceSelector in meas_algorithms and changed it to isParent test. Removed all references to nChild in unittests added through ticket DM-6824.

            Ticket passes both py2, py3, and ci_hsc jenkins builds. In addition to this validate_drp has been run and show to revert to the pervious value of 7.6756 rm in test AM1.

            Pull requests can be found at the following links:
            https://github.com/lsst/meas_algorithms/pull/62
            https://github.com/lsst/meas_astrom/pull/55
            https://github.com/lsst/pipe_tasks/pull/95

            Show
            cmorrison Chris Morrison added a comment - Removed from isMultiple test from matcherSourceSelector in meas_algorithms and changed it to isParent test. Removed all references to nChild in unittests added through ticket DM-6824 . Ticket passes both py2, py3, and ci_hsc jenkins builds. In addition to this validate_drp has been run and show to revert to the pervious value of 7.6756 rm in test AM1. Pull requests can be found at the following links: https://github.com/lsst/meas_algorithms/pull/62 https://github.com/lsst/meas_astrom/pull/55 https://github.com/lsst/pipe_tasks/pull/95
            Hide
            krughoff Simon Krughoff added a comment -

            This looks great to me. No comments. Merge away.

            Show
            krughoff Simon Krughoff added a comment - This looks great to me. No comments. Merge away.
            Hide
            cmorrison Chris Morrison added a comment -

            Finished review and pull request and pushed to master. Matching quality control in validate_drp should return to previous performance.

            Show
            cmorrison Chris Morrison added a comment - Finished review and pull request and pushed to master. Matching quality control in validate_drp should return to previous performance.

              People

              • Assignee:
                cmorrison Chris Morrison
                Reporter:
                cmorrison Chris Morrison
                Reviewers:
                Simon Krughoff
                Watchers:
                Chris Morrison, Simon Krughoff
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel