Details
-
Type:
RFC
-
Status: Implemented
-
Resolution: Done
-
Component/s: DM
-
Labels:None
Description
objectSizeStarSelector and secondMomentStarSelector (and their base class) currently take an exposure, from which they extract the detector (could be None) and the maskedImage (if making debugging plots). This suggests that what they really should take are two kwargs: detector and maskedImage, both defaulting to None.
This would simplify testing, as one wouldn't have to have or construct a full Exposure object when it's not really necessary. It also would give us good reason to clean up the selectStars methods to not have all the plotting code wedged inside.
One objection to this might be that secondMomentStarSelector calls algorithmsLib.makePsfCandidate which also takes an exposure. But PsfCandidate only uses getMaskedImage() and getXY0() (which it could get from the maskedImage).
More broadly speaking: is the intention that the star selectors work on source catalogs, or on images?
From cmorrison: After discussion with John Parejko we came up with the following class hierarchy for a new BaseSourceSelector. SourceSelectors that work on generic catalogs of objects (e.g. broader than just "stars") will inherit directly from the BaseSourceSelector class. BaseSourceSelector and its inherited classes must contain run() and selectSources() methods. The signature for selectSources will be a sourceCat, an optional maskedImage that could be None, and a field name sourceSelectedField that could be None. These methods will also take a matches variable for use in sourceSelectors that require matches (e.g. diaCatalogSourceSelector and catalogStarSelector). These sourceSelectors will raise and exception if matches is None. We will keep the usesMatches class variable and make it part of the base class. BaseStarSelector will inherit from BaseSouceSelector, with a config specific to selecting PSF-like objects.
| BaseSourceSelector ----------------------------> | BaseStarSelector |
| --> astrometrySourceSelector | --> meas_modelfit/S13StarSelector |
| --> matcherSourceSelector | --> meas_extenstions/psfexStarSelector |
| --> meas_astrom/catalogStarSelector | --> meas_algorithms/flaggedStarSelector |
| --> ip_diffim/diaCatalogSourceSelector | --> meas_algorithms/secondMomentStarSelector |
Some of the sourceSelectors listed (e.g. catalog, S13) do not appear to be used in the stack and could be deleted as part of this RFC.
Attachments
Issue Links
- is triggering
-
DM-9832 Cleanup and unify star selector call signatures
- Done
- relates to
-
DM-6901 Remove PsfCandidate code from second moment star selector
- Invalid
-
- Implemented
-
- Implemented
-
- Done
-
- Done
Activity
John Parejko what's the status of this RFC?
have explicitly added the usesMatches class variable to the plan. Unless someone has thought of a much better implementation or has major objections to the class structure as laid out, I assume this RFC is ready to be adopted as is.
John Parejko please add a "is triggering" work ticket to this RFC and adopt it.
Good points. I think that makes it clear that inheritance based on "uses-matches" also wouldn't work. But I still don't feel like we have a solution that handles this problem. Maybe just preserving the usesMatches class member is the best we can do, but I'd like to think we could come up with a better solution with a bit more thought.