Currently in the stack (as of w_2019_12) the AstrometryTask in meas_astrom has two configurations for a SourceSelector. The first is config.astrometry.sourceSelection (a configurable field for the subtask), and the second is config.astrometry.matcher.sourceSelector (a sourceSelectorRegistry). It turns out that this first one is used when the AstrometryTask is called with config.astrometry.forceKnownWcs=True (as in characterizeImage.py) and the second one is used when the full WCS fit is performed (in calibrate.py).
This is, at least, confusing (and certainly not documented), and possibly harmful when a user sets the wrong SourceSelector field.
In addition, AstrometryTask has a ReferenceSelector that is only used when config.astrometry.forceKnownWcs=True and is not used at all when fitting a full WCS. This incorrect behavior is addressed in
DM-18643. However, working on that ticket has led me down a path to propose this RFC.
Namely, I propose the following:
- The sourceSelector be removed from the matcher tasks (namely, MatchOptimisticBTask and MatchPessimisticBTask)
- The sourceSelection in AstrometryTask be changed to a SourceSelectorRegistry.
- The source selection and reference selection (fixing
DM-18643) will be called by AstrometryTask prior to the match-and-fit iterations. This will also make things (marginally) faster because in the current usage the (identical) source selection is repeated for each iteration.
This has the advantage that (a) users expect source and reference selectors to be together; (b) These selectors can go either in the matcher or AstrometryTask; (c) currently there's a source selector in matcher and a fake source and reference selector masquerading in the AstrometryTask; (d) I believe the proposed relocation to AstrometryTask makes more sense and the author of the matcher (Chris Morrison [X]) approves.
This will furthermore necessitate changing all the processCcd.py default configs in all the obs packages, as they all (to first order) make changes to the astrometry source selection settings.
Sounds fine to me. I would point out that the current structure of a sourceSelector in both RefMatch task and the matcher is uses introduces some likely unexpected behavior where two different sourceSelectors are run over the same data (once in RefMatch and once in the matcher).
Ah you mentioned this on slack and I only just noticed the code path that makes this happen:
That seems less than ideal, yes.
Adopting as-is since there were no complaints. I've changed
DM-18643 into the triggered ticket.