Performance cleanup for continuum background estimation

[jdbuhler]

The continuum background estimation code is documented as taking several hours to compute the background for a response with ~360 Em x phi bins. There are some obvious opportunities to rewrite the key parts of this code to eliminate data copies and interpreter loops. More importantly, the current implementation re-projects the same large, sparse binned data set for every Em / Phi combo; this is a very expensive operation that need be performed only once, rather than on every pass through the loop.

This patch overhauls the code to address the issues above. After my changes, the code spends around 45s loading and projecting the binned data set once, and then spends negligible time actually computing the background for all Em/Phi. The output should be identical to the previous code, except that I believe the old implementation of in-painting would not use the last pixel in the map because of an >= rather than a > in one test. Hopefully this implementation will be a source of inspiration as we consider how to implement better estimation algorithms.

Because the code is overall 2-3 orders of magnitude faster, there is no longer a need for progress-related printing and logging. I also removed the helper methods to load the response, which respectively consisted of two and one line of code that has nothing to do with the core task of background estimation. These operations will likely change anyway with the adoption of the interfaces branch, so let's just code them as needed in the notebook/script that calls background estimation, consistently with how we load responses in all the other tutorial notebooks.

I updated the background estimation notebook to compute every Em/Phi bin before writing the output.

I want to call out the fact that in this code, much like in the spectral fitting notebooks, projecting away the time axis from a large, sparse binned data set is a serious performance bottleneck. It would be much preferable to have the BinnedData object optionally bin events densely without using the time axis for analyses that don't need the event times. This would avoid the need for expensive projection later on.

[jdbuhler]

@ckarwin , should the first line of simple_inpainting compute the mean of only the nonzero entries in m_data? Your comment suggests that this is what you'd prefer.

[codecov]

Codecov Report

❌ Patch coverage is 94.73684% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (ebb34b1) to head (db122d6).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines	Patch %	Lines
...osipy/background_estimation/ContinuumEstimation.py	94.73%	3 Missing ⚠️

Files with missing lines	Coverage Δ
...osipy/background_estimation/ContinuumEstimation.py	95.94% <94.73%> (+53.19%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jdbuhler]

Additional question for @israelmcmc : should the opening and projection of the binned data be moved out of this class into the notebook? I assume you will want to use your new interfaces to open it.

[ckarwin]

@jdbuhler This entire continuum background estimation code has been replaced by a new version in an open PR to @israelmcmc interfaces branch: israelmcmc#10. The new version uses a a GCN for the inpainting, and it also includes the simplified inpainting method presented in the original version, which has been optimized and now takes on the order of seconds.

[jdbuhler]

Ah, ok. A quick look at the new code suggests that some variant of my vectorized inpainting code could still be useful in the non-NN case. But I will hold off on this until after interfaces and the new code goes in.

[israelmcmc]

Yeah, let's just wait a little bit. Thanks @jdbuhler!