Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: meas_algorithms, meas_astrom, pipe_tasks
-
Labels:None
-
Story Points:2
-
Epic Link:
-
Sprint:DRP X16-2
-
Team:Data Release Production
Description
The reference object loader in meas_algorithm's loadReferenceObjects.py grows the bbox by the config parameter pixelMargin: doc = "Padding to add to 4 all edges of the bounding box (pixels)" . This is set to 50 by default but is not reflected by the radius parameter set in the metadata, so some matches may reside outside the circle searched within this radius. This increase needs to be reflected in the radius set in the metadata fed into joinMatchListWithCatalog().
Attachments
Issue Links
- mentioned in
-
Page Loading...
Activity
Thank you, thank you, thank you for tracking this down! I've been aware of this bug for a few years, but never got to figuring out why it was happening, much less fixing it. Well done!
I put one little nit-pick comment on github (rewriting a for loop as a generator expression). Beyond that, I have one slightly more important comment: you access self.config.refObjLoader.pixelMargin in a few places, and I'm worried that you're breaking encapsulation by digging past the first layer of the Config. Do all loaders have a pixelMargin entry in their Config? If not, this will break when we retarget the loader. Ah, but I see it in the base class (LoadReferenceObjectsConfig in meas_algorithms)! So we're probably safe and you can ignore me.
Good stuff!
Paul Price
added a comment - Thank you, thank you, thank you for tracking this down! I've been aware of this bug for a few years, but never got to figuring out why it was happening, much less fixing it. Well done!
I put one little nit-pick comment on github (rewriting a for loop as a generator expression). Beyond that, I have one slightly more important comment: you access self.config.refObjLoader.pixelMargin in a few places, and I'm worried that you're breaking encapsulation by digging past the first layer of the Config . Do all loaders have a pixelMargin entry in their Config ? If not, this will break when we retarget the loader. Ah, but I see it in the base class ( LoadReferenceObjectsConfig in meas_algorithms)! So we're probably safe and you can ignore me.
Good stuff! Thanks for your enthusiastic review! I've adopted your suggestion to change the for loops into generator expressions in the instances in meas_astrom and meas_algorithms, pushed tickets/DM-5686 branches, reran Jenkins, and merged to master.
Should this issue be reflected in the W16/X16 release notes? Seems like it...
Done...sorry for the delay!
This broke the use of astrometry.net because the ANetAstrometryTask doesn't have a refObjLoader element (it's called solver). There's a fix in DM-5988.
Paul Price
added a comment - This broke the use of astrometry.net because the ANetAstrometryTask doesn't have a refObjLoader element (it's called solver ). There's a fix in DM-5988 .
Paul, would you mind giving this a review? The original error message I got was when trying to run the joinMatchListWithCatalog on a processCcd run of HSC's commissioning: --id visit=904028 ccd=9
afw.table WARNING: Persisted match record with ID 41874008000512 not found in catalog 1I fudged this by increasing the RADIUS in the match metadata before calling joinMatchListWithCatalog:
packedMatches = butler.get(dataset + "Match", dataRef.dataId)matchmeta = packedMatches.table.getMetadata()rad = matchmeta.getDouble("RADIUS")matchmeta.setDouble("RADIUS", rad*1.05, "field radius in degrees, approximate, padded")Here I fix the issue by explicitly including the pixelMargin padding to the createMatchMetadata function in meas_astrom so that the same search radius will be used when unpersisting as was used in the initial matching.
Unfortunately, I cannot reproduce the error as the newly refactored processCcd.py does not have the same reference selection criteria as the old one (which was used when the above issue was encountered), and the issue does not actually occur for this dataId any longer. I can confirm, however, that the radius is indeed getting set accordingly in the match metadata:
Without DM-5686
In [24]: matchmeta.getDouble("RADIUS")Out[24]: 0.0969113324124123With DM-5686
In [25]: matchmeta2.getDouble("RADIUS")Out[25]: 0.0995646234473273I then noticed that this was still not quite the same radius or center as used when the references are loaded. This is because the match metadata was being set after the exposure's wcs has been updated. So, in order to ensure that the same sky circle is set in the match metadata, I moved the creation of it to prior to any wcs-altering steps. So now the persisting and unperesisting sky circles match:
processCcd.calibrate.astrometry.refObjLoader: Loading reference objects using center (1023.5, 2087.5) pix = Fk5Coord(319.5213234, 0.1444358, 2000.00) sky and radius 0.109031123234 degMy debugging output from createMatchMetadata
createMatchMetadata: crtCoord = IcrsCoord(319.5213234, 0.1444358)createMatchMetadata: approxRadius = 0.109031123234Jenkins is happy.