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

nopytest_test_coadds.py throws warnings, and should be fixed

    Details

      Description

      nopytest_test_coadds.py throws the following warnings in many places:

      lsst.afw.image WARN: Could not parse options; writing with defaults:
        File "src/PropertySet.cc", line 189, in T lsst::daf::base::PropertySet::get(const string&) const [with T = std::shared_ptr<lsst::daf::base::PropertySet>; std::string = std::basic_string<char>]
          image not found {0}
      lsst::pex::exceptions::NotFoundError: 'image not found'
      

      This seems to be coming from:
      https://github.com/lsst/daf_persistence/blob/master/python/lsst/daf/persistence/posixStorage.py#L642
      which calls and executes to:
      https://github.com/lsst/afw/blob/a4679bc1e1d40d112e1484359a353fee477c1331/python/lsst/afw/image/image/fitsIoWithOptions.py#L97

      Where it expects a property of the property set to be called `image`. However, the options that are passed through the call chain are:
      `
      ['visit', 'ccd']
      `

      This optional prameter is generated:
      https://github.com/lsst/daf_persistence/blob/master/python/lsst/daf/persistence/posixStorage.py#L637
      and that variable comes from:
      https://github.com/lsst/daf_persistence/blob/master/python/lsst/daf/persistence/posixStorage.py#L249
      which in turn comes from the butler here:
      https://github.com/lsst/daf_persistence/blob/master/python/lsst/daf/persistence/butler.py#L1421
      the location variable is created here:
      https://github.com/lsst/daf_persistence/blob/master/python/lsst/daf/persistence/butler.py#L1297
      That optional data ultimately is initialized here:
      https://github.com/lsst/daf_persistence/blob/master/python/lsst/daf/persistence/butlerLocation.py#L207
      So whatever is creating butler locations (Some repository or mapper) is not adding the correct fields, or afw should be looking for different things to write.

      These are just notes for what I have found so far, to help anyone who gets to this ticket. I am tagging Paul Price because I think he was the last one to work with the additionalData code, and Jim Bosch because he may be familiar with this work, or how it impacts gen3 middleware and will know if this is even worth pursuing. Also tagging Yusra AlSayyad, as I am not sure if I put this in the right epic.

        Attachments

          Issue Links

            Activity

            nlust Nate Lust created issue -
            nlust Nate Lust made changes -
            Field Original Value New Value
            Epic Link DM-5346 [ 23141 ]
            nlust Nate Lust made changes -
            Risk Score 0
            Hide
            price Paul Price added a comment -

            fitsIoWithOptions.py was added by Russell Owen about a month ago as part of DM-15599.

            Show
            price Paul Price added a comment - fitsIoWithOptions.py was added by Russell Owen about a month ago as part of DM-15599 .
            Hide
            yusra Yusra AlSayyad added a comment -

            We're in Fall 2018, so type f18 and tab complete from there.

            Show
            yusra Yusra AlSayyad added a comment - We're in Fall 2018, so type f18 and tab complete from there.
            yusra Yusra AlSayyad made changes -
            Epic Link DM-5346 [ 23141 ] DM-14405 [ 79812 ]
            Hide
            jbosch Jim Bosch added a comment -

            I suspect this is just the non-CameraMapper Mapper in the  pipe.tasks.mocks simulation code not acting like a CameraMapper (and Butler expecting a CameraMapper even though it claims to just need Mapper).  That would make this less critical, as it means the test code is spewing unnecessary warnings but no production code is actually broken.  But that of course should be verified, and it should be fixed anyway.

            Show
            jbosch Jim Bosch added a comment - I suspect this is just the non-CameraMapper Mapper in the  pipe.tasks.mocks simulation code not acting like a CameraMapper (and Butler expecting a CameraMapper even though it claims to just need Mapper).  That would make this less critical, as it means the test code is spewing unnecessary warnings but no production code is actually broken.  But that of course should be verified, and it should be fixed anyway.
            Hide
            nlust Nate Lust added a comment -

            I meant more like tech debt or the like. It is not something that needs addressed right away. Might make for good pair programming or a diversion if someone needs a break.

            Show
            nlust Nate Lust added a comment - I meant more like tech debt or the like. It is not something that needs addressed right away. Might make for good pair programming or a diversion if someone needs a break.
            Hide
            Parejkoj John Parejko added a comment -

            I'm trying to debug this test right now (I've been modifying the internals of CoaddBase), and these warnings threw me off. A fix would be appreciated.

            Show
            Parejkoj John Parejko added a comment - I'm trying to debug this test right now (I've been modifying the internals of CoaddBase), and these warnings threw me off. A fix would be appreciated.
            jbosch Jim Bosch made changes -
            Assignee Jim Bosch [ jbosch ]
            Status To Do [ 10001 ] In Progress [ 3 ]
            Hide
            jbosch Jim Bosch added a comment -

            John Parejko, I think I've fixed the warnings described by Nate Lust on the tickets/DM-16082 of pipe_tasks if you want to rebase on that while debugging. 

            Yusra AlSayyad, I'm also seeing some warnings that seem to be related to PSF matching; a lot of psf-matched warps don't seem to be produced that are expected.  At least some of those are due to

            makeCoaddTempExp.warpAndPsfMatch INFO: Cannot PSF-Match: 
              File "src/math/Kernel.cc", line 197, in lsst::geom::Box2I lsst::afw::math::Kernel::shrinkBBox(const lsst::geom::Box2I&) const
                bbox dimensions = (22, 217) < (25, 25) in one or both dimensions {0}
            lsst::pex::exceptions::InvalidParameterError: 'bbox dimensions = (22, 217) < (25, 25) in one or both dimensions'
             

            ...but at first glance it doesn't look like there are enough of those to account for all of the failed gets at the end of the logs.

            Show
            jbosch Jim Bosch added a comment - John Parejko , I think I've fixed the warnings described by Nate Lust on the tickets/ DM-16082 of pipe_tasks if you want to rebase on that while debugging.  Yusra AlSayyad , I'm also seeing some warnings that seem to be related to PSF matching; a lot of psf-matched warps don't seem to be produced that are expected.  At least some of those are due to makeCoaddTempExp.warpAndPsfMatch INFO: Cannot PSF-Match: File "src/math/Kernel.cc" , line 197 , in lsst::geom::Box2I lsst::afw::math::Kernel::shrinkBBox( const lsst::geom::Box2I&) const bbox dimensions = ( 22 , 217 ) < ( 25 , 25 ) in one or both dimensions { 0 } lsst::pex::exceptions::InvalidParameterError: 'bbox dimensions = (22, 217) < (25, 25) in one or both dimensions' ...but at first glance it doesn't look like there are enough of those to account for all of the failed gets at the end of the logs.
            yusra Yusra AlSayyad made changes -
            Hide
            yusra Yusra AlSayyad added a comment -

            I'll double check every warp is accounted for in the morning and write a less scary looking error message for that scenario, but I don't expect a warp to be created for those visits. Those visits are no where near the patch.

            ds9 zscale calexp-0003*.fits deepCoadd_directWarp-00-1,1-0007.fits  & comparing with visit 7 just to get the quick outline. Top are the two calexps, and the bottom a rough boundary of 1,1.

            The next two lines offer more clues: directWarp has 0 good pixels (0.0%)

            makeCoaddTempExp INFO: Processing calexp 2 of 2 for this Warp: id=DataId(initialdata={'visit': 3, 'ccd': 2}, tag=set())
            makeCoaddTempExp.warpAndPsfMatch.psfMatch INFO: compute Psf-matching kernel
            makeCoaddTempExp.warpAndPsfMatch.psfMatch INFO: Padding Science PSF from (29, 29) to (35, 35) pixels
            makeCoaddTempExp.warpAndPsfMatch.psfMatch INFO: Adjusted dimensions of reference PSF model from (27, 27) to (35, 35)
            makeCoaddTempExp.warpAndPsfMatch.psfMatch INFO: Psf-match science exposure to reference
            makeCoaddTempExp.warpAndPsfMatch INFO: Cannot PSF-Match: makeCoaddTempExp.warpAndPsfMatch INFO: Cannot PSF-Match: 
              File "src/math/Kernel.cc", line 197, in lsst::geom::Box2I lsst::afw::math::Kernel::shrinkBBox(const lsst::geom::Box2I &) const
                bbox dimensions = (22, 217) < (25, 25) in one or both dimensions {0}
            lsst::pex::exceptions::InvalidParameterError: 'bbox dimensions = (22, 217) < (25, 25) in one or both dimensions'
             
            makeCoaddTempExp INFO: directWarp has 0 good pixels (0.0%)
            makeCoaddTempExp INFO: psfMatchedWarp has 0 good pixels (0.0%)
            

            Show
            yusra Yusra AlSayyad added a comment - I'll double check every warp is accounted for in the morning and write a less scary looking error message for that scenario, but I don't  expect a warp to be created for those visits. Those visits are no where near the patch. ds9 zscale calexp-0003 *.fits deepCoadd_directWarp-00-1,1-0007.fits  & comparing with visit 7 just to get the quick outline. Top are the two calexps, and the bottom a rough boundary of 1,1. The next two lines offer more clues: directWarp has 0 good pixels (0.0%) makeCoaddTempExp INFO: Processing calexp 2 of 2 for this Warp: id=DataId(initialdata={'visit': 3, 'ccd': 2}, tag=set()) makeCoaddTempExp.warpAndPsfMatch.psfMatch INFO: compute Psf-matching kernel makeCoaddTempExp.warpAndPsfMatch.psfMatch INFO: Padding Science PSF from (29, 29) to (35, 35) pixels makeCoaddTempExp.warpAndPsfMatch.psfMatch INFO: Adjusted dimensions of reference PSF model from (27, 27) to (35, 35) makeCoaddTempExp.warpAndPsfMatch.psfMatch INFO: Psf-match science exposure to reference makeCoaddTempExp.warpAndPsfMatch INFO: Cannot PSF-Match: makeCoaddTempExp.warpAndPsfMatch INFO: Cannot PSF-Match: File "src/math/Kernel.cc", line 197, in lsst::geom::Box2I lsst::afw::math::Kernel::shrinkBBox(const lsst::geom::Box2I &) const bbox dimensions = (22, 217) < (25, 25) in one or both dimensions {0} lsst::pex::exceptions::InvalidParameterError: 'bbox dimensions = (22, 217) < (25, 25) in one or both dimensions'   makeCoaddTempExp INFO: directWarp has 0 good pixels (0.0%) makeCoaddTempExp INFO: psfMatchedWarp has 0 good pixels (0.0%)
            Parejkoj John Parejko made changes -
            Link This issue relates to DM-15751 [ DM-15751 ]
            Hide
            Parejkoj John Parejko added a comment -

            Just to note: rebasing to this branch definitely cleaned up a lot of (I assume unnecessary) errors and warnings. There were still quite a few others that Yusra AlSayyad told me were expected, when I was debugging DM-15751.

            Show
            Parejkoj John Parejko added a comment - Just to note: rebasing to this branch definitely cleaned up a lot of (I assume unnecessary) errors and warnings. There were still quite a few others that Yusra AlSayyad told me were expected, when I was debugging DM-15751 .
            Hide
            jbosch Jim Bosch added a comment -

            John Parejko, mind a quick review, since you're already playing with these changes?

            Yusra AlSayyad, I think we've identified here that the other warnings are sufficiently different in nature that we should just merge what I've done here and consider that another ticket.

            Show
            jbosch Jim Bosch added a comment - John Parejko , mind a quick review, since you're already playing with these changes? Yusra AlSayyad , I think we've identified here that the other warnings are sufficiently different in nature that we should just merge what I've done here and consider that another ticket.
            jbosch Jim Bosch made changes -
            Reviewers John Parejko [ parejkoj ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            Hide
            Parejkoj John Parejko added a comment -

            See comments on comments. When I ran it, it was less complainy, so that's good.

            Show
            Parejkoj John Parejko added a comment - See comments on comments. When I ran it, it was less complainy, so that's good.
            Parejkoj John Parejko made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            jbosch Jim Bosch made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            swinbank John Swinbank made changes -
            Story Points 1

              People

              • Assignee:
                jbosch Jim Bosch
                Reporter:
                nlust Nate Lust
                Reviewers:
                John Parejko
                Watchers:
                Jim Bosch, John Parejko, Nate Lust, Yusra AlSayyad
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel