Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-11356

Confusion of refObjLoader.pixelMargin and matcher.maxOffsetPix

    XMLWordPrintable

    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

            Activity

            Hide
            price Paul Price added a comment - - edited

            Chris Morrison [X], would you please review these changes?

            price@pap-laptop:~/LSST/meas_algorithms (tickets/DM-11356=) $ git sub
            commit 691608fcb86d29bb7ff0e539d48ab7875d0b0609
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Mon Jul 24 13:23:43 2017 -0400
             
                LoadReferenceObjectsTask: add methods to provide metadata
                
                The 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 kept
                consistent: encapsulation.
             
             .../lsst/meas/algorithms/loadReferenceObjects.py   | 67 +++++++++++++++++++---
             1 file changed, 58 insertions(+), 9 deletions(-)
             
            commit f60ad38116bbad15c1ff7142e0889f23f08d8c83
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Mon Jul 24 13:24:34 2017 -0400
             
                LoadReferenceObjectsConfig: increase pixelMargin
                
                This 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 sub
            commit 642ffd06f558aa2f6e4d0a3546aace5979ad9a85
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Mon Jul 24 13:28:25 2017 -0400
             
                move createMatchMetadata functionality to LoadReferenceObjectsTask
                
                LoadReferenceObjectsTask 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 705d2327b115327aacd41e15420019db50ed810e
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Mon Jul 24 13:30:35 2017 -0400
             
                AstrometryTask: remove grow by maxOffsetPix
                
                This grow is already performed by the reference catalog loader, but it
                is controlled by the refObjLoader.pixelMargin parameter. That parameter
                now matches maxOffsetPix. Added a note to maxOffsetPix that the pixelMargin
                should 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(-)
            

            Show
            price Paul Price added a comment - - edited Chris Morrison [X] , would you please review these changes? price@pap-laptop:~/LSST/meas_algorithms (tickets/DM-11356=) $ git sub commit 691608fcb86d29bb7ff0e539d48ab7875d0b0609 Author: Paul Price <price@astro.princeton.edu> Date: Mon Jul 24 13:23:43 2017 -0400   LoadReferenceObjectsTask: add methods to provide metadata The 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 kept consistent: encapsulation.   .../lsst/meas/algorithms/loadReferenceObjects.py | 67 +++++++++++++++++++--- 1 file changed, 58 insertions(+), 9 deletions(-)   commit f60ad38116bbad15c1ff7142e0889f23f08d8c83 Author: Paul Price <price@astro.princeton.edu> Date: Mon Jul 24 13:24:34 2017 -0400   LoadReferenceObjectsConfig: increase pixelMargin This 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 sub commit 642ffd06f558aa2f6e4d0a3546aace5979ad9a85 Author: Paul Price <price@astro.princeton.edu> Date: Mon Jul 24 13:28:25 2017 -0400   move createMatchMetadata functionality to LoadReferenceObjectsTask LoadReferenceObjectsTask 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 705d2327b115327aacd41e15420019db50ed810e Author: Paul Price <price@astro.princeton.edu> Date: Mon Jul 24 13:30:35 2017 -0400   AstrometryTask: remove grow by maxOffsetPix This grow is already performed by the reference catalog loader, but it is controlled by the refObjLoader.pixelMargin parameter. That parameter now matches maxOffsetPix. Added a note to maxOffsetPix that the pixelMargin should 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(-)
            Hide
            price 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)
            

            Show
            price 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)
            Hide
            price 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(-)
            

            Show
            price 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(-)
            Hide
            price Paul Price added a comment -
            Show
            price Paul Price added a comment - Jenkins passed .
            Hide
            cmorrison Chris Morrison [X] (Inactive) added a comment - - edited

            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.

            Show
            cmorrison Chris Morrison [X] (Inactive) added a comment - - edited 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.
            Hide
            price Paul Price added a comment -

            Thanks Chris.

            Rebased everything against master, and re-running Jenkins.

            Show
            price Paul Price added a comment - Thanks Chris. Rebased everything against master, and re-running Jenkins .
            Hide
            price 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.

            Show
            price 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.
            Hide
            price Paul Price added a comment -

            Merged to master.

            Thanks Chris!

            Show
            price Paul Price added a comment - Merged to master. Thanks Chris!
            Hide
            danielsf Scott Daniel added a comment - - edited

            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.

            Show
            danielsf Scott Daniel added a comment - - edited 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.
            Hide
            danielsf Scott Daniel added a comment -

            ============================= 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 =====================
            

            Show
            danielsf Scott Daniel added a comment - ============================= 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 =====================
            Hide
            danielsf Scott Daniel added a comment -

            (It's going to be a while; my non-NFS machine has to rebuild afw)

            Show
            danielsf Scott Daniel added a comment - (It's going to be a while; my non-NFS machine has to rebuild afw)
            Hide
            price 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.

            Show
            price 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.
            Hide
            danielsf Scott Daniel added a comment -

            Okay. I opened DM-11810 (as feared, everything worked fine without NFS)

            Show
            danielsf Scott Daniel added a comment - Okay. I opened DM-11810 (as feared, everything worked fine without NFS)
            Hide
            tjenness Tim Jenness added a comment -

            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.

            Show
            tjenness Tim Jenness added a comment - 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.

              People

              Assignee:
              price Paul Price
              Reporter:
              price Paul Price
              Reviewers:
              Chris Morrison [X] (Inactive)
              Watchers:
              Chris Morrison [X] (Inactive), Hsin-Fang Chiang, John Swinbank, Paul Price, Scott Daniel, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.