Details
-
Type:
RFC
-
Status: Implemented
-
Resolution: Done
-
Component/s: DM
-
Labels:None
Description
Several failure modes have been found in the MatchOptimisticB blind astrometry matcher, specifically in dense fields with thousands of references objects to match to and similar numbers of detected sources. At these high densities, false positive matches for the Optimistic pattern matcher (OPMb) are no longer rare.
To address these problems we have created a more generalized version of the OPMb algorithm called MatchPesimisticB and which is described in DMTN-031. This DMTN also compares the performance of the two algorithms and shows that across a large dynamic range of reference and source densities MatchPesimisticB performs just as well as MatcherOptimisticB in low density environments and has much higher success rate for high densities. In terms of algorithm timing, MatchPesimisticB is roughly ~20% slower than MatchOptimisticB despite being implemented in Python vs MatchOptimisticB which is written in C++.
With these performance metrics in hand, this RFC would like to change the stack default algorithm to that of MatchPesimisticB instead of MatchOptimisticB for blind astrometry matching. This respresents a change to the default config of AstrometryTask in meas_astrom.
Attachments
Issue Links
Activity
I am very in favor of this. The new matcher is much better at setting the success criterion at an appropriate level over a wide range of source densities, and avoids many of the opaque tuning parameters that were present in the old matcher. We also used this in the crowded field testing effort and saw no significant issues over that range of densities.
Colin Slater
added a comment - I am very in favor of this. The new matcher is much better at setting the success criterion at an appropriate level over a wide range of source densities, and avoids many of the opaque tuning parameters that were present in the old matcher. We also used this in the crowded field testing effort and saw no significant issues over that range of densities. While I like the idea of making the astrometry more robust, I'm hesitant to wholeheartedly endorse a 20% slower algorithm, as I feel we have so many little inefficiencies which keep adding up. Can you do a quick profiling pass to see if there's any low-hanging fruit? Are there any plans to push the code down to C++?
Paul Price
added a comment - While I like the idea of making the astrometry more robust, I'm hesitant to wholeheartedly endorse a 20% slower algorithm, as I feel we have so many little inefficiencies which keep adding up. Can you do a quick profiling pass to see if there's any low-hanging fruit? Are there any plans to push the code down to C++? While I like the idea of making the astrometry more robust, I'm hesitant to wholeheartedly endorse a 20% slower algorithm, as I feel we have so many little inefficiencies which keep adding up. Can you do a quick profiling pass to see if there's any low-hanging fruit? Are there any plans to push the code down to C++?
Is the overall time spent matching actually significant? A 20% slowdown on something that takes up 20% of the total runtime of ProcessCcd would be worrying, as would a 20% slowdown on a change that didn't actually represent additional algorithmic sophistication. But worrying about a 20% slowdown on something that takes up (I'd guess) <5% of the total runtime of ProcessCcd because it's doing something more sophisticated might be premature optmization.
I'm not opposed to this, I'm just worried that we should be more concerned about performance than we historically have been: these additional 20%s add up. I think the ten minutes it would take to run a profile would be a good investment (in personal enrichment if nothing else).
Paul Price
added a comment - I'm not opposed to this, I'm just worried that we should be more concerned about performance than we historically have been: these additional 20%s add up. I think the ten minutes it would take to run a profile would be a good investment (in personal enrichment if nothing else). I understand the concern. However, I'm not terribly worried about the 20%. If you look at Tubar07, for the density of objects they run, most of the processing is spent creating the searchable data structures on the reference catalog and this is still true of the new algorithm. Currently the we run several match/fit cycles in the code but we recreate these look-ups for each step in the cycle. Allowing these structures to be reused in a given match/fit cycle would be fairly easy to implement for MatchPessimisticB and would eliminate any slow down for the code compared to the current algorithm.
As for running porting any of the slower portions of the code, such as creating the reference look-up tables or the main loop over candidate reference pairs (the two definite high pegs for very dense fields), I have talked previous with others at UW about writing these in C++. The plan was to look into it after this RFC was accepted.
I support this RFC.
Michael Wood-Vasey
added a comment - I support this RFC. Hi everyone, it looks like this RFC is on it's way to adoption. I'll give it till tomorrow (Wednesday) morning before I hit adopted and open the tickets needed to make it the default. I talked with John Swinbank and we decided to also include the speed up to the pessimistic matcher code that I described above.