DM-49302: Make GetTemplateTask more memory efficient

[isullivan]

These changes include keeping the input coadds as deferredDatasetHandles until individual tracts are assembled, cleaning up variables in loops, and using indices instead of full arrays in a few places.

Edit: The other PR has merged, so now only commits from this ticket are on this PR

[parejkoj]

Several comments; I'm concerned about some of these changes causing either failures or incorrect behavior in ways the current tests won't catch. I don't know for sure that it's wrong, but I'm worried.

I also don't think the dels provide any real benefit.

[parejkoj]

I don't think these can hurt, but they may not reduce memory usage here (the fact that they're C++ objects under the hood also complicates matters).

Maybe put an explicit note above the dels about why they're deleted here?

[isullivan]

I'l certainly add a comment. In my memory profiling, if I leave all of the dels out the total memory usage is ~500MB higher.

[parejkoj]

Please do add that comment.

[parejkoj]

ooof, is this really necessary? That's unfortunate. I'd have thought there was an easier way to do this in the middleware, but maybe not if we don't have a real butler?

If you lifted this from elsewhere, is there a place we could put it that both could use instead of having it be copied?

[isullivan]

The original that I based this on is in lsst.cell_coadds. I certainly don't want to add a dependency on that package here, and my version is modified from that one. This code could be refactored and added to utils or pipe_base, but I would like to do that on a different ticket if possible.

[parejkoj]

I don't think this does anything, because it gets overwrittten it immediately at the start of the loop, so the old array will go out of scope.

[isullivan]

It doesn't need to be in the for loop, and the comment is misleading. The memory savings is from the weight from the last iteration being deleted before new arrays are created after the loop. This saves ~200MB from the peak memory use in my profiling.

[parejkoj]

Ah, I was thinking average memory, but I could see it shaving a bit off the peak.

[parejkoj]

I think this is a logic change that is incorrect. good here is a bitmask and I don't know that the assignment below (maskedImage.image.array[good] *= weight) will behave correctly. I also am not sure the tests properly explore the cases here (e.g. bad variance on the edge and in the middle).

I don't know what numpy's behavior is when assigning to a bitmasked-array in this manner, and I'd be a bit surprised if it was fully self-consistent with the old logic.

[isullivan]

maskedImage.variance.array[good] returns a numpy array of only the values of maskedImage.variance.array where good is True. The later assignment maskedImage.image.array[good] *= weight does indeed update just the elements of maskedImage.image.array that correspond to indices where good is True. This is a feature of numpy arrays, which should be stable and reliable.

[parejkoj]

Ah, ok. As I think about it more, since it's a 1d array under the hood, I guess the logic holds. I was just surprised, as I don't think I'd seen this style of usage before.

[parejkoj]

Please add the comment where you delete a couple images, otherwise this looks good. Hopefully it saves us enough memory and doesn't slow anything down!