Hey Gabor — could you please add a story point estimate when you get a moment? Thanks!

John Swinbank : "Moved" 20 points from the parent issue.

The notebook looks great, and suggests that the new code is calculating the decorrelation correctly. Please see the additional comments on the pull requests.

Some points to note here copy pasted from my github response:

When you return a Struct with only one element, you should think about whether you really want the Struct at all.

This is a result of dropping the correction kernel from the return. I am not 100% sure that we don't want to return any additional information; I'll postpone the removal of the Struct for the spatially varying correction implementation DM-23857. Added TODO comment to the task description and to the return statement.
Considerations: It'd be logical to return something about the correction kernel itself, but...

• returning the Fourier space correction needs the freqSpaceShape and may not suit for any outside operation.
• Returning the image space correction (full image size) needs one additional, otherwise unnecessary, IFT. Also where the origin is placed would need detailed description or additional, otherwise unnecessary, copying (re-centering) of the array.
• If a matchingKernel size cropped version is returned then we throw away the main lesson from this whole change i.e. that the correction can be larger than the original matching kernel in general so should not be used.

Please add a comment describing why 1e8 was chosen. Also, since this is only a warning, you might want to consider lowering the threshold. Finally, I'm a bit surprised by this check: isn't the template variance usually smaller than the science image, rather than the other way around?

As the matching kernel high frequency components go to zero, the correction factor converges to sqrt(tvar/svar) i.e. this is a warning when high frequency pixels of the difference image is multiplied by more than 4 orders of magnitude as correction. Yes, in the regular case, the template variance is the smaller one and this is no concern; tvar>svar can happen if we convolve the science image thus the arguments are swapped for this task.
I don't have a really good justification for the value itself at the moment, just wanted an indicator to watch for diverging solutions. We won't coadd ~1e8 images, so I don't expect that any realistic coadd variance will be this small (i.e. this large when swapped in science exposure convolution mode) but this scaling is still acceptable at double precision (I mean assuming within one Fourier space image to have order ~1. values at low frequency and ~1e4 values at high frequencies and having ~1e7 pixels still fits in the double precision conveniently for any calculation/transformation. )

Re taking the real part in DecorrelateALKernelTask.computeCorrection:

kft2 and mk2 (below) should both be real based on their calculation, but I suspect they will still be stored as complex. I think you want to explicitly take the real part of both to force them to be used as floats.

My original consideration here was that converting them to real then back to complex when applying the returned correction may take more time. But this is not the case, np.real creates a view and taking the sqrt of reals is much faster.

See lsst-dm/diffimTests : tickets/DM-24371_decorr_afterburner_tests/DM-24371_real_complex_conversion_timing.ipynb

Re in-place modification, computeCorrectedDiffimPsf actually did not do in-place modification but this was indeed a bug in computeCorrectedImage. Added making of a copy internally; we can remove it later if this becomes a performance bottleneck. Thanks Ian Sullivan for spotting this!

If you haven't already, can you file a bug ticket describing the problem in computeCorrectedImage?