Implementing the code in DM-5532 brought up a few issues that I would like feedback on.

Some stars may be rejected when making PSF candidates (is this usually due to inadequate selection, or inherent?). Thus by not always making PSF candidates we lose some suitability information. If this is important, it can be ameliorated by making run create the candidate list, which could down-select the star catalog and flag only the final stars.

At the other extreme, if we don't care about the fact that some stars cannot be made into good PSF candidates when selecting stars, then makePsfCandidates does not seem to be a natural method of star selectors. It uses two config parameters that are not needed by any star selector (except the second moment star selector, which also makes PSF candidates while selecting stars). To me it makes more sense for this to be a method of PSF determiners (e.g. implemented in a base class, and have the main method accept a star catalog), but that would expand the scope of this RFC. It could also be added to MeasurePsfTask or become standalone code. If it is added to PSF determiners then I'd be tempted to give them the same treatment as star selectors: a base class that is a task.

Every star selector seems to have its own set of bad flags used to reject sources as unsuitable. Why the disparity? Can we reduce or eliminate it (which will change behavior)? This could be done in DM-5532 or on another ticket.

Note that reneged on keeping the star selector registry because we have no nice way to make subtasks from a registry; the constructor is called with too little information. In the long term we may want to implement subtask registries, since it would let us group related subtasks.

I think making PSF candidates should be simpler, so I filed DM-5578. That is not meant to be part of this RFC, I'm just noting it.

Making StarSelectors use a consistent set of flags by default sounds like a good idea (as long as the set of flags remains configurable), and I'd definitely endorse making the way those flags are configured consistent.. I suspect just adopting the flags used by ObjectSizeStarSelector is probably what we want. Should probably be a separate issue, though.

I'm much more concerned about this extra selection done by PsfCandidate - we need to understand exactly what PsfCandidate is doing, and probably move that logic to Python if we want to move forward with this issue. Having additional (opaque) selection criteria imposed as side-effect to PsfCandidate construction seems like a design flaw that we'd like to fix, but we can't just drop the behavior and hope it isn't important.

If we have the run method make PSF candidates then we can easily obtain the old behavior and return both a star catalog and an associated set of PSF candidates (both the same length in the same order), thus exactly preserving the old behavior even for users that only want stars (of which we don't yet have any – I had a use case, but it turned out to be unnecessary). This will remove extensive code duplication (the part that makes PSF candidates) and better isolate algorithm-specific code in one method (selectStars or, if we want to avoid having users accidentally select stars that cannot be made into PSF candidates, make it a private method).

In any case, I agree it would be useful to understand why stars are being selected that cannot be made into PSF candidates. I'm trying to find some cases. I also agree that unifying flags may be best left for a separate, cleanup ticket. It has the potential to change behavior, so will require plenty of testing. It may be or may not be related to causes of failure for making PSF candidates.

No luck, so far, triggering failure of stars to be made into PSF candidates. None with the quick test for CFHT or DECam in validate_drp. I'll try the longer tests, but I think this is not worth holding up the RFC and code over.

I do think it is important to report such errors more vocally than some of the star selectors were doing. The object size star selector (at least) was logging at debug level. I would rather report these as warnings (and if we get a flood of such message it would be easy to consolidate them down to a single report, or one per type of failure).

So I propose the following:

• Make StarSelector an abstract base task with three methods: selectStars, makePsfCandidates and run. The only abstract method is selectStars
• Have run select stars and make a PSF candidate list and only return and flag stars that can be made into PSF candidates. This makes it very easy to use for PSF fitting (which is our current dominant use case), and assures that the list of PSF candidates and catalog of stars will match. At the same time you can still call selectStars to only get a star catalog, and subclasses need only override that one method.

I have implemented that in DM-5532 and it seems to work nicely. Further refinements may be possible, but it is much nicer than the old code in that:

• It is easy to get a star catalog
• It reduces code duplication

Adopted as follows:

• Make star selectors tasks
• Eliminate the registry, for now, in lieu of a registry field for subtasks (one that passes arguments such as log to the constructor)
• Add an abstract base class for star selectors with the following methods:
• selectStars abstract method that takes a catalog of sources and returns an lsst.pipe.base.Struct containing a catalog of stars
• makePsfCandidates a concrete method that takes a catalog of stars (as returned by selectStars and produces PSF candidates; it logs a warning for any star that cannot be turned into a PSF candidate, and so returns an lsst.pipe.base.Struct a list of PSF candidates and (in the same order) a catalog of stars that were successfully made into PSF candidates
• run a concrete method that takes a catalog of sources and an optional name of a flag field and calls selectStars, then makePsfCandidates, then optionally sets the flags of the final star catalog