DM-54555: Move bright star stamps code into pipe_tasks

[leeskelvin]

No description provided.

[timj]

I would have liked to be involved a bit in the design discussion here because I don't like that meas_algorithms now has two types of stamps and some of those inherit from the shared base class and others are completely independent. There was a concept of a generic stamps hierarchy with a trampoline so that people didn't need to know in advance which type of stamp they were serializing. Am I missing that this still exists?

[leeskelvin]

Happy to talk more, but I was keen to make progress on this ticket and not hold up progress on our bright star wings effort. As far as I know, there isn't an lsst.images implementation for storage of postage stamps yet, is there?

If you have plans to write and support something more generic with lsst.images, I'd be happy to use that, but I'd rather not stall bright star wings for too long.

As bright star stamps haven't yet been widely adopted, switching to lsst.images now seemed like the obvious choice, before mass usage of this new class begins.

[TallJimbo]

I very much want to separate the bright star use case from the cutout service (and I think that's true of most other "stamp" use cases); I think the apparent similarities are really incidental duplication. But I take @timj's point about confusion, especially within a single package. If we can separate the names I think that would help a lot.

[timj]

I created the Stamps class to act as a generic hierarchy for handling collections of MaskedImage with unified I/O schemes and allowing metadata extensions for specializations. This concept disappears if we now write a separate independent Stamps-like class for each specialization. I understand that inheritance vs traits might not have been the best choice but I don't think the idea is a bad one that people have some unified model for how we utilize collections of masked images.

[leeskelvin]

These are highly-specialized collections of stamps however. They've been warped, calibrated, had their mask planes modified, and we attach the PSF kernel image alongside each one. FWIW, following a discussion with @TallJimbo on this earlier, I'm currently working on moving this code out of meas_algorithms to sit alongside the remaining bright star code in pipe_tasks. That would at least prevent meas_algorithms having two types of stamp defined.

[timj]

Ok. Your description still sounds consistent with my concept for Stamps. I'm sorry I didn't catch this discussion earlier.

[leeskelvin]

Yes, likewise for me when you discussed this yesterday. If you'd rather keep *stamps under a single base class definition, I can live with that. Might I suggest we rename these to BrightStarImage (single) and BrightStarCollection (multiple) instead? Names can be workshopped.

[TallJimbo]

My concern is that if we have similarities now and factor them into a common base class, if that factorization needs to change in the future (because needs diverge), then both classes/format break (or need extra work to avoid breakage) instead of only one. I am sympathetic to the idea that it's good for users if the interfaces are as similar as possible (just so people don't have to remember two different ways of writing similar things), but I really don't the implementations or file formats coupled too tightly.

[timj]

It was designed to have a pretty loose coupling. The reader code was generic because it would look at the FITS headers to work out the Stamps specialization class and then ask that class to load the file. The writer code also gave people a way to specialize whilst calling a base class that made sure you wrote all the EXTNAME/EXTVER headers correctly (which is critically important for Stamps and people easily get wrong).

[timj]

I assume that in lsst.images we would have a concept in the FITS serializer for asking for a Stamp at a time so that the HDUs can be written with correct EXTNAME/EXTVER with EXTVER incrementing for each Stamp.

[leeskelvin]

I can't speak to the future plans for stamps in lsst.images.

What is in scope for this ticket is what we do prior to testing of bright star subtraction. I don't want to begin large scale testing in an old format that will quickly become outdated. I'd much prefer to have these data output in lsst.images format from the beginning to minimize future churn.

If your concern @timj is potential confusion to the end-user that these may be considered a form of "stamp", I've already suggested above that I'm happy to switch to a different naming convention. Bright star cutouts are intended to be an internal dataset for consumption by BrightStarStackTask alone.