# estimateBackground should not make a deep copy of the exposure

## Details

• Type: Improvement
• Status: Done
• Priority: Major
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Templates:
• Story Points:
4
• Sprint:
• Team:

## 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

## Activity

Hide
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
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
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
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
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
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
Russell Owen added a comment -
Show
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:
Russell Owen
Reporter:
Russell Owen
Reviewers:
Paul Price
Watchers:
Paul Price, Russell Owen