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

Write `_construct_pattern_and_shift_rot_matrix` in C++/pybind

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_astrom
    • Labels:
      None
    • Story Points:
      10
    • Sprint:
      AP F21-6 (November), AP S22-1 (December)
    • Team:
      Alert Production
    • Urgent?:
      No

      Description

      Write C++ code wrapped with pybind11 to replace the `_construct_pattern_and_shift_rot_matrix`, associated loops, and sub-methods.

        Attachments

          Issue Links

            Activity

            Hide
            cmorrison Chris Morrison [X] (Inactive) added a comment - - edited

            Ran through the RC2 dataset as used in previous tickets. No additional detectors were lost due to slight changes in the algorithm. Below are the timings in seconds.

            This ticket:
            Mean: 1.271 STD: 0.499
            Percentiles: 5%,  25%,  50%,  75%,     95%
                             0.870, 1.039, 1.194, 1.407, 1.884
            Min: 0.635, Max: 28.494

            Compared to the previous timings in DM-32010, this represents a ~60% reduction in the mean time and a ~10 fold decrease in scatter.

            Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35590/pipeline

            The code as committed and tested represents a slight change in algorithm. I've moved the python intermediate_verify step outside of the the main loop over reference candidates (now in the C++ layer). This was done as coding the least squares fit in C++ would take to long compared to the time I have left on my employment. There are no additional failures caused by this change.

            There are likely specific speed ups still available in the code, specifically in the create_sorted_arrays method. That method uses std::vectors and push back to create it's arrays which is inefficient for creating the sorted distance array and it's argsort.

            Show
            cmorrison Chris Morrison [X] (Inactive) added a comment - - edited Ran through the RC2 dataset as used in previous tickets. No additional detectors were lost due to slight changes in the algorithm. Below are the timings in seconds. This ticket: Mean: 1.271 STD: 0.499 Percentiles: 5%,  25%,  50%,  75%,     95%                  0.870, 1.039, 1.194, 1.407, 1.884 Min: 0.635, Max: 28.494 Compared to the previous timings in DM-32010 , this represents a ~60% reduction in the mean time and a ~10 fold decrease in scatter. Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35590/pipeline The code as committed and tested represents a slight change in algorithm. I've moved the python intermediate_verify step outside of the the main loop over reference candidates (now in the C++ layer). This was done as coding the least squares fit in C++ would take to long compared to the time I have left on my employment. There are no additional failures caused by this change. There are likely specific speed ups still available in the code, specifically in the create_sorted_arrays method. That method uses std::vectors and push back to create it's arrays which is inefficient for creating the sorted distance array and it's argsort.
            Hide
            cmorrison Chris Morrison [X] (Inactive) added a comment -

            Sending it to you Ian as I won't be able to review the full ticket. Jenkins is failing but in pipe_base which looks like possibly a break on master. meas_astrom passes.

            Show
            cmorrison Chris Morrison [X] (Inactive) added a comment - Sending it to you Ian as I won't be able to review the full ticket. Jenkins is failing but in pipe_base which looks like possibly a break on master. meas_astrom passes.
            Hide
            cmorrison Chris Morrison [X] (Inactive) added a comment -

            Added link to ticket DM-32985 which regards implementing the intermediate_verify fit and fully match the Python code. However, this is optional as I do not believe the fitting step here in the loop as any significant effect on the algorithm as implemented.

            Show
            cmorrison Chris Morrison [X] (Inactive) added a comment - Added link to ticket DM-32985 which regards implementing the intermediate_verify fit and fully match the Python code. However, this is optional as I do not believe the fitting step here in the loop as any significant effect on the algorithm as implemented.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Code looks good; I just realized Ian Sullivan probably didn't get the GitHub notification.

            Show
            krzys Krzysztof Findeisen added a comment - Code looks good; I just realized Ian Sullivan probably didn't get the GitHub notification.
            Hide
            sullivan Ian Sullivan added a comment -

            I didn't, thanks for pointing it out to me.

            Show
            sullivan Ian Sullivan added a comment - I didn't, thanks for pointing it out to me.
            Show
            Parejkoj John Parejko added a comment - Jenkins: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/35839/pipeline

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              cmorrison Chris Morrison [X] (Inactive)
              Reviewers:
              Krzysztof Findeisen
              Watchers:
              Chris Morrison [X] (Inactive), Eric Bellm, Ian Sullivan, John Parejko, Krzysztof Findeisen
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins Builds

                  No builds found.