The proposed new task structure (which is missing some details; a closer but evolving version can be found in DM-4692) makes it hard to measure aperture correction. The issue is that the source catalog must contain a flag field indicating if a source was used to measure PSF, but the proposed CalibrateTask will not have that information, since it never tries to measure PSF.
I can see several ways out of this. Feedback, including alternate suggestions, would be most welcome.
Move final detection and measurement into CharacterizeImageTask
CharacterizeImageTask already has the “used for PSF” flag from the brighter catalog, so it can easily perform the deeper detection and final measurement that includes aperture correction, e.g. by copying the flag from the shallower catalog. This leaves CalibrateTask to simply run AstrometryTask and PhotoCalTask.
One side benefit: a common testing need is to avoid operations that require a reference catalog. In this plan CalibrateTask only performs such operations so it can simply be elided for such tests. If CalibrateTask continues to perform the final detection and measurement then it is a bit harder to elide only the operations that require a reference catalog. On the other hand, if CalibrateTask does so little it may no longer be needed.
Perform PSF fitting in CalibrateTask
If we add a PSF fitting step in CalibrateTask then it gains all the information it needs to measure aperture correction. The question is whether it is safe and desirable to fit a PSF based on a deeper catalog. Certainly our current code does this, but the change to using only bright stars to fit the PSF was intentional.
Implement CalibrateTask as proposed and pass it “used for PSF” information
I dislike this solution because it makes the CalibrateTask less self-contained and much harder to use (at least when measurement of aperture correction is wanted).
Here’s another option: I don’t think we need to rely on the “used for PSF” to measure the aperture corrections, which makes it pretty straightforward to just put aperture correction measurement in CalibrateTask. Once we have a PSF model, it’s actually much easier to select stars, and that should allow us to put together a better filter for what to use for aperture correction than what we’d get from just using the “used for PSF” flag. We’d then need to introduce a new flag to indicate stars used in aperture correction, of course. And we’d need a place (maybe a new subtask?) to put the code that does the selection of those stars - it’d be nice to ultimately see how much we can put that into the same interface as the StarSelector interface used for PSF determination, but right now I think they’re too different (and StarSelector is too tied up with PsfCandidate) for that to be worthwhile.
Does your solution mean we would not have to copy “used for PSF” and similar flags from the initial brighter source catalog to the final deeper source catalog?
Regarding using StarSelector as the API…would it be so terrible to return a list of PsfCandidates, especially initially? They are not exactly what we want, but seem fairly lightweight, being consisting of a few pointers and scalars.
Note that a star selector that used an initial PSF model might be useful in CharacterizeImageTask (especially if it didn’t need too precise a PSF model); such a star selector would be a good choice for all but the first iteration of detect/measure sources/measure PSF.
In any case…could you please describe the algorithm you had in mind?
Correct - at least while we’re producing the final, deeper source catalog. It’s still true that linking those two catalogs at some point (probably before database ingest) is important for QA and debugging.
My recollection is that PsfCandidates are quite a pain to construct, and in general I’d rather remove them from StarSelector than use them in selecting stars for aperture correction. But given that it’s temporary I don’t have a strong preference.
Agreed. I hadn’t considered the possibility that we’d just want to add a previous PSF model to the StarSelector interface. It sounds like it might be a good idea (though I’ll leave it up to you whether we want to consider that now or later).
I had two ideas:
PSF mag - CModel mag, plus a cut on magnitude (this is what goes into extendedness). Problem with this is that it would require running CModel on the full set of fainter objects, which might be expensive. But we do need to run CModel on at least the objects we select for aperture correction measurement anyway, and that might be a pain to do unless we run it on the full set of fainter objects.
sqrt((ixx - pxx) + (iyy - pyy)), plus a cut on magnitude. ixx, iyy are adaptive moments of the source and pxx, pyy are adaptive moments of the PSF (SdssShape at least computes the former; if it doesn’t compute the latter, it will after changes are merged from HSC).
The existing star selector API already can accept an initial PSF model because it accepts an exposure. However, I don’t believe any current star selectors take advantage of that fact.
I’ve looked through some star selectors and it looks to me as if their code would be simpler if they returned something simpler than a list of PsfCandidates. I have not found any instances where it would be inefficient to first find all stars, then make a list of PSF candidates. In particular, the complicated ObjectSizeStarSelector first computes the list of stars, then computes PSF candidates.
I propose that StarSelector.selectStars be changed to do one of two things:
Return a list of sources or source IDs
Set a flag in a list of sources. This is usually wanted, but if not then it is a pain for users because a flag field and a reference to it must be provided. (If star selectors were tasks this would be more natural).
It would be easy enough to provide code that converted the returned list to a list of PsfCandidates. If that operation is star-selector-specific (and I don’t see why it should be) then it could be a new method on StarSelector.
All this said, I am tempted to make it a different ticket and just put up with PSF candidates for now. I really want to finish the main overhaul.
That proposal sounds fine to me, including deferring the switch to avoid PsfCandidate in StarSelector to another ticket (if that’s what you think will be easier).
Great. The code is all pushed, but I don’t yet have a unit test for CalibrateTask (so it could easily be broken), and the existing test for ProcessCcdTask doesn’t run it due to not having a reference catalog. (And I’m not copying the flags from the bright source catalog – that’ll come)