Details

    • Type: Story
    • Status: Done
    • Priority: Major
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Templates:
    • Story Points:
      3
    • Team:
      Data Facility

      Description

      Remove pex_logging dependency on pipe_tasks.

        Issue Links

          Activity

          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          All required dependencies of pipe_tasks no long use pex_logging. Two optional dependencies, meas_deblender and ip_diffIm, still use pex_logging.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - All required dependencies of pipe_tasks no long use pex_logging. Two optional dependencies, meas_deblender and ip_diffIm, still use pex_logging.
          Show
          hchiang2 Hsin-Fang Chiang added a comment - Jenkins: https://ci.lsst.codes/job/stack-os-matrix/label=centos-7,python=py2/18159//console and ci_hsc: https://ci.lsst.codes/job/stack-os-matrix/label=centos-7,python=py2/18160//console
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          David Reiss may you please review this? Most changes are in ip_diffim, where pex.logging multi-level tracing was used in many places. In the commit message I tried to explain how I mapped to the new scheme; let me know if they make sense to you.

          Some of the examples in ip_diffim and meas_deblender are very outdated and cannot run. That problem is partially captured in DM-7838.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - David Reiss may you please review this? Most changes are in ip_diffim , where pex.logging multi-level tracing was used in many places. In the commit message I tried to explain how I mapped to the new scheme ; let me know if they make sense to you. Some of the examples in ip_diffim and meas_deblender are very outdated and cannot run. That problem is partially captured in DM-7838 .
          Hide
          reiss David Reiss added a comment -

          I have gone through all of your changes to the logging in the three packages. Other than a couple of very minor comments, it otherwise looks fine.

          Show
          reiss David Reiss added a comment - I have gone through all of your changes to the logging in the three packages. Other than a couple of very minor comments, it otherwise looks fine.
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          Were you able to compare the logging output of the examples (at least the ones that were able to be run) and ensure that the output of the new code is sufficiently similar to the old output?

          I was not able to run most of the examples in ip_diffim; some gave AssertionError with leaked blocks, some had problems opening the afwdata files, and so on. But I was able to run examples/snapPsfMatchTask.py, and these are the last 10 lines of the output:

          Before:

               lsst.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Background = 100.869
             lsst.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Candidate 18 resids = -0.103 +/- 1.017 sigma (529 pix)
              lsst.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Candidate 18 core resids = -0.281 +/- 1.052 sigma (49 pix)
          psfMatch INFO: Final spatial kernel sum 0.959
               lsst.ip.diffim.KernelSolution.getConditionNumber DEBUG: EIGENVALUE eMax / eMin = 3.483e+05
          psfMatch INFO: Spatial model condition number 3.483e+05
          psfMatch INFO: Doing stats of kernel candidates used in the spatial fit.
          psfMatch INFO: 9 candidates total, 0 rejected, 9 used
          psfMatch INFO: Spatial kernel model well constrained; 9 candidates, 1 terms, 21 bases
          psfMatch INFO: Spatial background model appears well constrained; 9 candidates, 1 terms
          

          After:

          TRACE4.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Background = 100.460
          TRACE2.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Candidate 18 resids = 0.036 +/- 1.034 sigma (529 pix)
          TRACE3.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Candidate 18 core resids = -0.052 +/- 0.988 sigma (49 pix)
          psfMatch INFO: Final spatial kernel sum 0.973
          TRACE3.ip.diffim.KernelSolution.getConditionNumber DEBUG: EIGENVALUE eMax / eMin = 3.228e+05
          psfMatch INFO: Spatial model condition number 3.228e+05
          psfMatch INFO: Doing stats of kernel candidates used in the spatial fit.
          psfMatch INFO: 9 candidates total, 0 rejected, 9 used
          psfMatch INFO: Spatial kernel model well constrained; 9 candidates, 1 terms, 21 bases
          psfMatch INFO: Spatial background model appears well constrained; 9 candidates, 1 terms
          

          The default format of the new logging doesn't prepend spaces for those tracing/debugging records.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - Were you able to compare the logging output of the examples (at least the ones that were able to be run) and ensure that the output of the new code is sufficiently similar to the old output? I was not able to run most of the examples in ip_diffim ; some gave AssertionError with leaked blocks, some had problems opening the afwdata files, and so on. But I was able to run examples/snapPsfMatchTask.py , and these are the last 10 lines of the output: Before: lsst.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Background = 100.869 lsst.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Candidate 18 resids = - 0.103 +/- 1.017 sigma ( 529 pix) lsst.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Candidate 18 core resids = - 0.281 +/- 1.052 sigma ( 49 pix) psfMatch INFO: Final spatial kernel sum 0.959 lsst.ip.diffim.KernelSolution.getConditionNumber DEBUG: EIGENVALUE eMax / eMin = 3 .483e+ 05 psfMatch INFO: Spatial model condition number 3 .483e+ 05 psfMatch INFO: Doing stats of kernel candidates used in the spatial fit. psfMatch INFO: 9 candidates total, 0 rejected, 9 used psfMatch INFO: Spatial kernel model well constrained; 9 candidates, 1 terms, 21 bases psfMatch INFO: Spatial background model appears well constrained; 9 candidates, 1 terms After: TRACE4.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Background = 100.460 TRACE2.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Candidate 18 resids = 0.036 +/- 1.034 sigma ( 529 pix) TRACE3.ip.diffim.AssessSpatialKernelVisitor.processCandidate DEBUG: Candidate 18 core resids = - 0.052 +/- 0.988 sigma ( 49 pix) psfMatch INFO: Final spatial kernel sum 0.973 TRACE3.ip.diffim.KernelSolution.getConditionNumber DEBUG: EIGENVALUE eMax / eMin = 3 .228e+ 05 psfMatch INFO: Spatial model condition number 3 .228e+ 05 psfMatch INFO: Doing stats of kernel candidates used in the spatial fit. psfMatch INFO: 9 candidates total, 0 rejected, 9 used psfMatch INFO: Spatial kernel model well constrained; 9 candidates, 1 terms, 21 bases psfMatch INFO: Spatial background model appears well constrained; 9 candidates, 1 terms The default format of the new logging doesn't prepend spaces for those tracing/debugging records.
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          Thank you for your review, David!

          The 3 branches are now merged.

          With these merged, pipe_tasks no longer uses pex_logging, shown here.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - Thank you for your review, David! The 3 branches are now merged. With these merged, pipe_tasks no longer uses pex_logging , shown here .

            People

            • Assignee:
              hchiang2 Hsin-Fang Chiang
              Reporter:
              hchiang2 Hsin-Fang Chiang
              Reviewers:
              David Reiss
              Watchers:
              David Reiss, Hsin-Fang Chiang
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development