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

Fix order of operations when using temporary local backgrounds in detection

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_algorithms
    • Labels:
    • Story Points:
      3
    • Sprint:
      DRP S17-5
    • Team:
      Data Release Production

      Description

      Our current implementation of the temporary local background approach to avoiding spurious detections near bright objects simply subtracts a local background from the full image before performing any detection steps. That can result in missed isolated-object detections and incorrect Footprints for large objects.

      Instead, we should:
      1. Detect Footprints and Peaks.
      2. Subtract the local background.
      3. Detect Peaks within each Footprint again, and use the new set of Peaks instead of the old set if and only if there is at least one Peak in the new set.

      This really ought to be fixed before the HSC internal release or major HSC processing at NCSA.

        Attachments

          Issue Links

          blocks

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-10129 Process HSC RC dataset using Stack version chosen for the full S17B reprocessing

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-10266 Release hscPipe 5.0-beta4

          • Done
          is triggering

          Bug - A problem which impairs or prevents the functions of the product. DM-10336 DM-10271 seems to have broken afw

          • Done

            Activity

            Ascending order - Click to sort in descending order
            4 older comments
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            processCcd.py on one CCD, before this change:

            real	2m30.659s
            		 
            user	2m10.405s
            		 
            sys	0m25.486s

            and after:

            real	3m17.279s
            		 
            user	2m17.502s
            		 
            sys	0m26.649s

            Show
            jbosch Jim Bosch added a comment - processCcd.py on one CCD, before this change: real 2m30.659s user 2m10.405s sys 0m25.486s and after: real 3m17.279s user 2m17.502s sys 0m26.649s
            Hide
            Permalink
            price Paul Price added a comment -

            I've made some comments on the GitHub PRs, including suggestions for optimisation.

            Show
            price Paul Price added a comment - I've made some comments on the GitHub PRs, including suggestions for optimisation.
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            Performance regression fixed; the time to run detection on my test coadd patch has gone from 162s down 3.6s. I'm now doing the thresholding for the peaks only in the subimage covered by the footprint whose peaks we're trying to replace, which cuts down on both the comparison between footprints and the amount of image we have to threshold again (especially because I'm not trying to replace the peaks of footprints with only one peak).

            It looks like processCcd time spent on detection was always in the noise; my latest benchmark is:

            real	3m38.712s
            		 
            user	2m12.946s
            		 
            sys	0m25.709s

            Paul Price, if you're free to take another look today, please do, but I'll go ahead and merge later today if I don't hear from you as I believe I've addressed your biggest concern.

            Show
            jbosch Jim Bosch added a comment - Performance regression fixed; the time to run detection on my test coadd patch has gone from 162s down 3.6s. I'm now doing the thresholding for the peaks only in the subimage covered by the footprint whose peaks we're trying to replace, which cuts down on both the comparison between footprints and the amount of image we have to threshold again (especially because I'm not trying to replace the peaks of footprints with only one peak). It looks like processCcd time spent on detection was always in the noise; my latest benchmark is: real 3m38.712s user 2m12.946s sys 0m25.709s Paul Price , if you're free to take another look today, please do, but I'll go ahead and merge later today if I don't hear from you as I believe I've addressed your biggest concern.
            Hide
            Permalink
            price Paul Price added a comment -

            Had a quick look, and made some trivial comments on the GitHub PR.

            Show
            price Paul Price added a comment - Had a quick look, and made some trivial comments on the GitHub PR.
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            Merged to master. Thanks for the very helpful review, Paul Price (and Lauren MacArthur).

            Show
            jbosch Jim Bosch added a comment - Merged to master. Thanks for the very helpful review, Paul Price (and Lauren MacArthur ).

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Bob Armstrong, John Swinbank, Paul Price
              Watchers:
              Bob Armstrong, Jim Bosch, John Swinbank, Paul Price, Robert Lupton
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.