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

estimateBackground should not make a deep copy of the exposure

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_algorithms
    • Labels:
      None

      Description

      Implement RFC-155: change estimateBackground as follows:

      • Always subtract the background
      • Modify the exposure in place
      • Replace estimateBackground with the run method of a new task SubtractBackgroundTask
      • Replace getBackground (which fits a background) with SubtractBackgroundTask.fitBackground

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment - - edited

            Most of the work is in meas_algorithms, but it affected some code in ip_diffim, pipe_drivers and pipe_tasks. It also required a trivial change to a comment in meas_base.

            Show
            rowen Russell Owen added a comment - - edited Most of the work is in meas_algorithms , but it affected some code in ip_diffim , pipe_drivers and pipe_tasks . It also required a trivial change to a comment in meas_base .
            Hide
            price Paul Price added a comment -

            Review completed on GitHub PRs. Besides some little things, the only major comment I have is that the SubtractBackgroundTask.run method should be made as compact as possible by delegating work to other methods.

            Show
            price Paul Price added a comment - Review completed on GitHub PRs. Besides some little things, the only major comment I have is that the SubtractBackgroundTask.run method should be made as compact as possible by delegating work to other methods.
            Hide
            rowen Russell Owen added a comment -

            Thank you for the comprehensive review. It was very helpful.

            I agree that computing statistics on the background is a poor idea for this task, but it appears to be necessary for now, so I carried it over from existing code and filed RFC-180. I also agree that the background subtraction in `ip_diffim` could be cleaned up a bit, but I feel that would be best left for a more comprehensive refactoring. I was wrong about the tight coupling between the task's config and `makeBackground`'s control object, so I updated the documentation accordingly.

            Aside from that, I believe I implemented all of your suggested changes. I also modernized the display code in `SubtractBackgroundTask` and rebased from master. Once CI passes I will merge.

            Show
            rowen Russell Owen added a comment - Thank you for the comprehensive review. It was very helpful. I agree that computing statistics on the background is a poor idea for this task, but it appears to be necessary for now, so I carried it over from existing code and filed RFC-180 . I also agree that the background subtraction in `ip_diffim` could be cleaned up a bit, but I feel that would be best left for a more comprehensive refactoring. I was wrong about the tight coupling between the task's config and `makeBackground`'s control object, so I updated the documentation accordingly. Aside from that, I believe I implemented all of your suggested changes. I also modernized the display code in `SubtractBackgroundTask` and rebased from master. Once CI passes I will merge.
            Hide
            rowen Russell Owen added a comment -
            Show
            rowen Russell Owen added a comment - The changes are described here: https://community.lsst.org/t/changes-in-how-background-subtraction-is-done/756

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Paul Price
              Watchers:
              Paul Price, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.