Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: meas_astrom
-
Labels:None
-
Team:External
Description
A recent change in AstrometryTask.solve increases the exposure bounding box by the matcher's maximum offset (matcher.maxOffsetPix. However, a parameter already exists to provide this growth in the reference catalog loader (refObjLoader.pixelMargin). Because the exposure bounding box is increased artificially after recording the catalog loading radius, loading the normalised persisted matches results in many missed matches.
To fix this:
- Remove the additional grow
- Set the default refObjLoader.pixelMargin to match the default matcher.maxOffsetPix.
- Add notes advising the user that the two parameters should be linked.
- Stop AstrometryTask from reaching into the responsibility of the reference catalog loader (it shouldn't need to know about the details of the refObjLoader configuration).
Attachments
Issue Links
- relates to
-
- Done
Activity
An additional change was required in meas_extensions_astrometryNet:
price@pap-laptop:~/LSST/meas_extensions_astrometryNet (tickets/DM-11356=) $ git sub-patch
|
commit f0fa7db3775a1aa2c14f530c0a04ec3e1db3d0bc
|
Author: Paul Price <price@astro.princeton.edu>
|
Date: Mon Jul 24 14:24:52 2017 -0400
|
|
|
tests: set LoadReferenceObjectsConfig.pixelMargin to former value
|
|
The default value has been modified; set it to what it used to be so that
|
the tests can pass.
|
|
|
diff --git a/tests/testLoadAstrometryNetObjects.py b/tests/testLoadAstrometryNetObjects.py
|
index 95d7fd5..87396f2 100755
|
--- a/tests/testLoadAstrometryNetObjects.py
|
+++ b/tests/testLoadAstrometryNetObjects.py
|
@@ -44,6 +44,7 @@ class TestLoadAstrometryNetObjects(unittest.TestCase):
|
# Set up local astrometry_net_data
|
self.datapath = setupAstrometryNetDataDir('photocal')
|
self.config = LoadAstrometryNetObjectsTask.ConfigClass()
|
+ self.config.pixelMargin = 50 # Original default when these tests were written
|
|
self.bbox = afwGeom.Box2I(afwGeom.Point2I(0, 0), afwGeom.Extent2I(3001, 3001))
|
self.ctrPix = afwGeom.Point2I(1500, 1500)
|
Paul Price
added a comment - An additional change was required in meas_extensions_astrometryNet:
price@pap-laptop:~/LSST/meas_extensions_astrometryNet (tickets/DM-11356=) $ git sub-patch
commit f0fa7db3775a1aa2c14f530c0a04ec3e1db3d0bc
Author: Paul Price <price@astro.princeton.edu>
Date: Mon Jul 24 14:24:52 2017 -0400
tests: set LoadReferenceObjectsConfig.pixelMargin to former value
The default value has been modified; set it to what it used to be so that
the tests can pass.
diff --git a/tests/testLoadAstrometryNetObjects.py b/tests/testLoadAstrometryNetObjects.py
index 95d7fd5..87396f2 100755
--- a/tests/testLoadAstrometryNetObjects.py
+++ b/tests/testLoadAstrometryNetObjects.py
@@ -44,6 +44,7 @@ class TestLoadAstrometryNetObjects(unittest.TestCase):
# Set up local astrometry_net_data
self.datapath = setupAstrometryNetDataDir('photocal')
self.config = LoadAstrometryNetObjectsTask.ConfigClass()
+ self.config.pixelMargin = 50 # Original default when these tests were written
self.bbox = afwGeom.Box2I(afwGeom.Point2I(0, 0), afwGeom.Extent2I(3001, 3001))
self.ctrPix = afwGeom.Point2I(1500, 1500)
And pipe_tasks:
price@pap-laptop:~/LSST/pipe_tasks (tickets/DM-11356=) $ git sub
|
commit f7ffba699a9300b70f279853d85d818bf5ebb603
|
Author: Paul Price <price@astro.princeton.edu>
|
Date: Mon Jul 24 14:40:51 2017 -0400
|
|
|
CalibrateTask: remove createMatchMeta
|
|
The task of creating the match metadata has been moved to its principal
|
consumer: LoadReferenceObjectsTask. Some slight restructuring was required
|
to adjust here.
|
|
|
python/lsst/pipe/tasks/calibrate.py | 13 +++++--------
|
1 file changed, 5 insertions(+), 8 deletions(-)
|
Paul Price
added a comment - And pipe_tasks:
price@pap-laptop:~/LSST/pipe_tasks (tickets/DM-11356=) $ git sub
commit f7ffba699a9300b70f279853d85d818bf5ebb603
Author: Paul Price <price@astro.princeton.edu>
Date: Mon Jul 24 14:40:51 2017 -0400
CalibrateTask: remove createMatchMeta
The task of creating the match metadata has been moved to its principal
consumer: LoadReferenceObjectsTask. Some slight restructuring was required
to adjust here.
python/lsst/pipe/tasks/calibrate.py | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
Paul Price
added a comment - Jenkins passed . Sorry for the delay, Paul. The change in the meta data storage looks clear enough to me. Updates to the matcher configs also clearly state that the refloader config should be edited as well. Assuming this still passes Jenkins I am happy to have the code merged.
Thanks Chris.
Rebased everything against master, and re-running Jenkins.
Paul Price
added a comment - Thanks Chris.
Rebased everything against master, and re-running Jenkins . Jenkins passed (two failures due to ci_hsc not working with py3, unrelated to this work). I will now merge to master.
Paul Price
added a comment - Jenkins passed (two failures due to ci_hsc not working with py3, unrelated to this work). I will now merge to master. Merged to master.
Thanks Chris!
Paul Price
added a comment - Merged to master.
Thanks Chris! I just had meas_algorithms fail. I will attach the *.failed file below. It was passing this morning. I think the only thing that changed was that this ticket got merged.
The build was on an NFS-mounted Linux machine. I am testing on a non-NFS machine to see if this is another NFS-specific bug.
============================= test session starts ==============================
|
platform linux -- Python 3.5.2, pytest-3.2.0, py-1.4.34, pluggy-0.4.0
|
rootdir: /astro/users/danielsf/lsstsw3/build/meas_algorithms, inifile:
|
plugins: session2file-0.1.9, xdist-1.19.2.dev0+g459d52e.d20170831, forked-0.3.dev0+g1dd93f6.d20170831, flake8-0.8.1
|
gw0 I / gw1 I / gw2 I / gw3 I / gw4 I / gw5 I / gw6 I / gw7 I / gw8 I / gw9 I / gw10 I / gw11 I / gw12 I / gw13 I / gw14 I / gw15 I / gw16 I / gw17 I / gw18 I / gw19 I / gw20 I / gw21 I / gw22 I / gw23 I / gw24 I / gw25 I / gw26 I / gw27 I / gw28 I / gw29 I / gw30 I / gw31 I
|
gw0 [162] / gw1 [162] / gw2 [162] / gw3 [162] / gw4 [162] / gw5 [162] / gw6 [162] / gw7 [162] / gw8 [162] / gw9 [162] / gw10 [162] / gw11 [162] / gw12 [162] / gw13 [162] / gw14 [162] / gw15 [162] / gw16 [162] / gw17 [162] / gw18 [162] / gw19 [162] / gw20 [162] / gw21 [162] / gw22 [162] / gw23 [162] / gw24 [162] / gw25 [162] / gw26 [162] / gw27 [162] / gw28 [162] / gw29 [162] / gw30 [162] / gw31 [162]
|
|
|
scheduling tests via LoadScheduling
|
.....................................................................................................................F............................................
|
generated xml file: /astro/users/danielsf/lsstsw3/build/meas_algorithms/tests/.tests/pytest-meas_algorithms.xml
|
=================================== FAILURES ===================================
|
__________________ SpatialModelPsfTestCase.testCandidateList ___________________
|
[gw5] linux -- Python 3.5.2 /astro/users/danielsf/lsstsw3/miniconda/bin/python
|
|
|
self = <test_psfIO.SpatialModelPsfTestCase testMethod=testCandidateList>
|
|
|
def setUp(self):
|
width, height = 100, 300
|
self.mi = afwImage.MaskedImageF(afwGeom.ExtentI(width, height))
|
self.mi.set(0)
|
self.mi.getVariance().set(10)
|
self.mi.getMask().addMaskPlane("DETECTED")
|
|
self.FWHM = 5
|
self.ksize = 25 # size of desired kernel
|
|
self.exposure = afwImage.makeExposure(self.mi)
|
|
psf = roundTripPsf(2, algorithms.DoubleGaussianPsf(self.ksize, self.ksize,
|
self.FWHM/(2*math.sqrt(2*math.log(2))), 1, 0.1))
|
self.exposure.setPsf(psf)
|
|
for x, y in [(20, 20),
|
# (30, 35), (50, 50),
|
(60, 20), (60, 210), (20, 210)]:
|
|
flux = 10000 - 0*x - 10*y
|
|
sigma = 3 + 0.01*(y - self.mi.getHeight()/2)
|
> psf = roundTripPsf(3, algorithms.DoubleGaussianPsf(self.ksize, self.ksize, sigma, 1, 0.1))
|
|
|
tests/test_psfIO.py:111:
|
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
|
|
|
key = 3
|
psf = <lsst.meas.algorithms.doubleGaussianPsf.DoubleGaussianPsf object at 0x7fca627e56f8>
|
|
|
def roundTripPsf(key, psf):
|
global psfFileNum
|
pol = policy.Policy()
|
additionalData = dafBase.PropertySet()
|
|
if psfFileNum % 2 == 1:
|
storageType = "Boost"
|
else:
|
storageType = "Xml"
|
loc = dafPersist.LogicalLocation(
|
"tests/data/psf%d-%d.%s" % (psfFileNum, key, storageType))
|
psfFileNum += 1
|
persistence = dafPersist.Persistence.getPersistence(pol)
|
|
storageList = dafPersist.StorageList()
|
storage = persistence.getPersistStorage("%sStorage" % (storageType), loc)
|
storageList.append(storage)
|
persistence.persist(psf, storageList, additionalData)
|
|
storageList2 = dafPersist.StorageList()
|
> storage2 = persistence.getRetrieveStorage("%sStorage" % (storageType), loc)
|
E RuntimeError: unrecognized XML syntax
|
|
|
tests/test_psfIO.py:78: RuntimeError
|
==================== 1 failed, 161 passed in 40.90 seconds =====================
|
(It's going to be a while; my non-NFS machine has to rebuild afw)
Scott Daniel: The error in tests/test_psfIO.py has nothing to do with this work. I suggest you file a new ticket and/or ask on Slack.
Paul Price
added a comment - Scott Daniel : The error in tests/test_psfIO.py has nothing to do with this work. I suggest you file a new ticket and/or ask on Slack. Okay. I opened DM-11810 (as feared, everything worked fine without NFS)
Paul Price regarding your ci_hsc py3 failure, if you want you can do a build with tickets/DM-8560-scons and that will allow you to test everything on py3.
Chris Morrison [X], would you please review these changes?
price@pap-laptop:~/LSST/meas_algorithms (tickets/DM-11356=) $ git subcommit 691608fcb86d29bb7ff0e539d48ab7875d0b0609Author: Paul Price <price@astro.princeton.edu>Date: Mon Jul 24 13:23:43 2017 -0400LoadReferenceObjectsTask: add methods to provide metadataThe LoadReferenceObjectsTask is the consumer of what has been called the'match metadata', because its 'joinMatchListWithCatalog' method reads it.Therefore, it should also produce it, so that the format can be keptconsistent: encapsulation..../lsst/meas/algorithms/loadReferenceObjects.py | 67 +++++++++++++++++++---1 file changed, 58 insertions(+), 9 deletions(-)commit f60ad38116bbad15c1ff7142e0889f23f08d8c83Author: Paul Price <price@astro.princeton.edu>Date: Mon Jul 24 13:24:34 2017 -0400LoadReferenceObjectsConfig: increase pixelMarginThis values matches MatchOptimisticBConfig.maxOffsetPix .python/lsst/meas/algorithms/loadReferenceObjects.py | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)price@pap-laptop:~/LSST/meas_astrom (tickets/DM-11356=) $ git subcommit 642ffd06f558aa2f6e4d0a3546aace5979ad9a85Author: Paul Price <price@astro.princeton.edu>Date: Mon Jul 24 13:28:25 2017 -0400move createMatchMetadata functionality to LoadReferenceObjectsTaskLoadReferenceObjectsTask is the consumer of the 'match metadata',and therefore, to maintain encapsulation, it should produce it.python/lsst/meas/astrom/__init__.py | 1 -python/lsst/meas/astrom/astrometry.py | 9 ++++--python/lsst/meas/astrom/createMatchMetadata.py | 43 --------------------------python/lsst/meas/astrom/directMatch.py | 3 +-python/lsst/meas/astrom/ref_match.py | 8 +++--5 files changed, 14 insertions(+), 50 deletions(-)commit 705d2327b115327aacd41e15420019db50ed810eAuthor: Paul Price <price@astro.princeton.edu>Date: Mon Jul 24 13:30:35 2017 -0400AstrometryTask: remove grow by maxOffsetPixThis grow is already performed by the reference catalog loader, but itis controlled by the refObjLoader.pixelMargin parameter. That parameternow matches maxOffsetPix. Added a note to maxOffsetPix that the pixelMarginshould be coordinated.python/lsst/meas/astrom/astrometry.py | 5 +----.../lsst/meas/astrom/matchOptimisticB/matchOptimisticBContinued.py | 3 ++-python/lsst/meas/astrom/matchPessimisticB.py | 3 ++-3 files changed, 5 insertions(+), 6 deletions(-)