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

Footprint dilation performance regression

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Labels:
      None

      Description

      In DM-1128 we implemented span-based dilation for footprints. A brief test on synthetic data indicated that this was a performance win over the previous version of the code.

      In May 2015, this code was merged to HSC and applied to significant quantities of real data for the first time. A major performance regression was identified:

      [May-9 00:26] Paul Price: processCcd is now crazy slow.
      [May-9 00:29] Paul Price: Profiling...
      [May-9 00:40] Paul Price: I'm thinking it's the Footprint grow code...
      [May-9 00:44] Paul Price: And the winner is…. Footprint construction:
      [May-9 00:44] Paul Price: 2 0.000 0.000 702.280 351.140 /home/astro/hsc/products/Linux64/meas_algorithms/HSC-3.8.0/python/lsst/meas/algorithms/detection.py:191(makeSourceCatalog)
 2 0.005 0.002 702.274 351.137 /home/astro/hsc/products/Linux64/meas_algorithms/HSC-3.8.0/python/lsst/meas/algorithms/detection.py:228(detectFootprints)
 15 0.001 0.000 698.597 46.573 /home/pprice/hsc/afw/python/lsst/afw/detection/detectionLib.py:3448(_init_)
 15 698.596 46.573 698.596 46.573 {_detectionLib.new_FootprintSet}
      [May-9 00:53] Paul Price: If I revert HSC-1243 ("Port better Footprint-grow code from LSST"), then the performance regression goes away. @jbosch @jds may be interested...

      The source of the regression must be identified and resolved for both HSC and LSST.

        Attachments

          Activity

          Hide
          swinbank John Swinbank added a comment - - edited

          Paul writes:

          it was at the very first part of the pipeline — processCcd.py, in the detection part. So you should be able to test it on the LSST side

          and

          Aha! I've found the log file with the slow processing, and the slowest was visit=11690 ccd=49:

          2015-05-09T04:11:12: processExposure: tiger-r11n14:27449: Start processing {'taiObs': '2014-11-18', 'pointing': 1052, 'visit': 11690, 'dateObs': '2014-11-18', 'filter': 'HSC-G', 'field': 'SSP_UDEEP_COSMOS', 'ccd': 49, 'expTime': 300.0} (ccdId=2338049)
          [...]
          2015-05-09T04:24:11: processExposure: tiger-r11n14:27449: Finished writing CCD {'taiObs': '2014-11-18', 'pointing': 1052, 'visit': 11690, 'dateObs': '2014-11-18', 'filter': 'HSC-G', 'field': 'SSP_UDEEP_COSMOS', 'ccd': 49, 'expTime': 300.0}

          (13 minutes!)

          Show
          swinbank John Swinbank added a comment - - edited Paul writes: it was at the very first part of the pipeline — processCcd.py, in the detection part. So you should be able to test it on the LSST side and Aha! I've found the log file with the slow processing, and the slowest was visit=11690 ccd=49: 2015-05-09T04:11:12: processExposure: tiger-r11n14:27449: Start processing {'taiObs': '2014-11-18', 'pointing': 1052, 'visit': 11690, 'dateObs': '2014-11-18', 'filter': 'HSC-G', 'field': 'SSP_UDEEP_COSMOS', 'ccd': 49, 'expTime': 300.0} (ccdId=2338049) [...] 2015-05-09T04:24:11: processExposure: tiger-r11n14:27449: Finished writing CCD {'taiObs': '2014-11-18', 'pointing': 1052, 'visit': 11690, 'dateObs': '2014-11-18', 'filter': 'HSC-G', 'field': 'SSP_UDEEP_COSMOS', 'ccd': 49, 'expTime': 300.0} (13 minutes!)
          Hide
          swinbank John Swinbank added a comment -

          Running on tiger-sumire with the current (f6a3296b0b) version of LSST's afw, I have:

          $ time processCcd.py /tigress/HSC/HSC/ --output=/tigress/HSC/HSC/rerun/swinbank/testing/ --id visit=11690 ccd=49 --clobber-config
          [...]
          real    0m58.217s
          user    0m42.895s
          sys     0m3.694s
          

          Reverting all the RLE-related growth operations in the afw, I have:

          real    0m52.768s
          user    0m39.109s
          sys     0m2.941s
          

          Re-running both of the above a few times, it's obvious the scatter is pretty high – neither is consistently faster than the other.

          Paul Price, am I misunderstanding your suggestion? If not, I'll try setting up an HSC stack and see if I can replicate the problem there.

          Show
          swinbank John Swinbank added a comment - Running on tiger-sumire with the current ( f6a3296b0b ) version of LSST's afw , I have: $ time processCcd.py /tigress/HSC/HSC/ --output=/tigress/HSC/HSC/rerun/swinbank/testing/ --id visit=11690 ccd=49 --clobber-config [...] real 0m58.217s user 0m42.895s sys 0m3.694s Reverting all the RLE-related growth operations in the afw , I have: real 0m52.768s user 0m39.109s sys 0m2.941s Re-running both of the above a few times, it's obvious the scatter is pretty high – neither is consistently faster than the other. Paul Price , am I misunderstanding your suggestion? If not, I'll try setting up an HSC stack and see if I can replicate the problem there.
          Hide
          swinbank John Swinbank added a comment -

          Despite my failure to replicate the problem running on real data, there is a performance regression here. The RLE dilation algorithm creates a very large number of spans, then relies on Footprint::normalize() to sort them all out in the end. That normalization process is slow.

          We can avoid this by keeping the proto-Footprint in a normal form as we create it. I produced a proof of concept implementation of this which seems to work reasonably well. Given a simplistic benchmark, I have:

          Implementation Small non-isotropic Small isotropic Medium non-isotropic Medium isotropic Large non-isotropic Large isotropic
          Original 0.009737 0.006181 0.7547 2.573 8.655 40.72
          Naive RLE 0.001688 0.002375 0.2481 0.8749 17.14 18.07
          Optimized RLE 0.001647 0.001977 0.02680 0.05380 0.2494 0.2472

          Where the numbers are time to grow a single circular footprint in seconds, and "small", "medium" and "large" correspond to growing footprints of diameter 10, 100 and 300 pixels by 2, 20 and 30 pixels respectively.

          This analysis is pretty crude, but it's clear that the optimized form is a significant win over both previous versions for large footprints.

          I'm curious as to whether this change alleviates the problem seen on HSC.

          Show
          swinbank John Swinbank added a comment - Despite my failure to replicate the problem running on real data, there is a performance regression here. The RLE dilation algorithm creates a very large number of spans, then relies on Footprint::normalize() to sort them all out in the end. That normalization process is slow. We can avoid this by keeping the proto-Footprint in a normal form as we create it. I produced a proof of concept implementation of this which seems to work reasonably well. Given a simplistic benchmark , I have: Implementation Small non-isotropic Small isotropic Medium non-isotropic Medium isotropic Large non-isotropic Large isotropic Original 0.009737 0.006181 0.7547 2.573 8.655 40.72 Naive RLE 0.001688 0.002375 0.2481 0.8749 17.14 18.07 Optimized RLE 0.001647 0.001977 0.02680 0.05380 0.2494 0.2472 Where the numbers are time to grow a single circular footprint in seconds, and "small", "medium" and "large" correspond to growing footprints of diameter 10, 100 and 300 pixels by 2, 20 and 30 pixels respectively. This analysis is pretty crude, but it's clear that the optimized form is a significant win over both previous versions for large footprints. I'm curious as to whether this change alleviates the problem seen on HSC.
          Hide
          swinbank John Swinbank added a comment -

          Even with the HSC stack, I'm unable to replicate the problem.

          Specifically, I have done:

          $ . /tigress/HSC/products/eups/master/bin/setups.sh
          $ setup hscPipe 3.7.3
          $ setup -j astrometry_net_data ps1_pv2_20150302
          $ git clone https://github.com/HyperSuprime-Cam/afw.git
          $ cd afw
          $ setup -j -r .
          $ git checkout HSC-3.9.0
          $ scons opt=3
          $ time hscProcessCcd.py /tigress/HSC/HSC --rerun swinbank/footprint --id visit=11690 ccd=49  --clobber-config
          [...]
          real	3m7.212s
          user	2m47.708s
          sys	0m4.261s
          $ git checkout HSC-3.9.1
          $ scons opt=3
          $ time hscProcessCcd.py /tigress/HSC/HSC --rerun swinbank/footprint --id visit=11690 ccd=49  --clobber-config
          [...]
          real	3m8.881s
          user	2m45.779s
          sys	0m4.075s
          

          Where HSC-3.9.0 includes the new (slow?) Footprint growth code as merged to HSC by Jim, and HSC-3.9.1 contains the Paul/RHL reversions of same.

          I'm a bit stumped as to how to reproduce the significant regression Paul Price saw. Paul, do you have any further suggestions? Is my inexperience with the pipeline causing me to miss something obvious?

          Show
          swinbank John Swinbank added a comment - Even with the HSC stack, I'm unable to replicate the problem. Specifically, I have done: $ . /tigress/HSC/products/eups/master/bin/setups.sh $ setup hscPipe 3.7.3 $ setup -j astrometry_net_data ps1_pv2_20150302 $ git clone https://github.com/HyperSuprime-Cam/afw.git $ cd afw $ setup -j -r . $ git checkout HSC-3.9.0 $ scons opt=3 $ time hscProcessCcd.py /tigress/HSC/HSC --rerun swinbank/footprint --id visit=11690 ccd=49 --clobber-config [...] real 3m7.212s user 2m47.708s sys 0m4.261s $ git checkout HSC-3.9.1 $ scons opt=3 $ time hscProcessCcd.py /tigress/HSC/HSC --rerun swinbank/footprint --id visit=11690 ccd=49 --clobber-config [...] real 3m8.881s user 2m45.779s sys 0m4.075s Where HSC-3.9.0 includes the new (slow?) Footprint growth code as merged to HSC by Jim, and HSC-3.9.1 contains the Paul/RHL reversions of same. I'm a bit stumped as to how to reproduce the significant regression Paul Price saw. Paul, do you have any further suggestions? Is my inexperience with the pipeline causing me to miss something obvious?
          Hide
          price Paul Price added a comment -

          Maybe I got the ccd number wrong. Try visit=11690 ccd=4:

          $ eups list -s
          afw                   LOCAL:/tigress/pprice/dm-2787/afw         setup
          astrometry_net        0.50              v9_1 HSC-latest b163 setup
          astrometry_net_data   ps1_pv2_20150302  setup
          base                  HSC-3.0.0f_hsc    HSC-latest setup
          boost                 1.55.0            b3 HSC HSC-latest setup
          cfitsio               3.360             HSC HSC-latest setup
          clapack               3.2.1             current HSC-latest setup
          coadd_utils           HSC-3.0.0l_hsc    HSC-latest setup
          daf_base              HSC-3.1.0         HSC-latest setup
          daf_butlerUtils       HSC-3.3.0e_hsc    HSC-latest setup
          daf_persistence       HSC-3.1.0         HSC-latest setup
          distEst               HSC-3.0.0l_hsc    HSC-latest setup
          doxygen               1.8.5             sims b824 n20150327_b949 b780 2014_09 HSC b660 b649 b261 b858 qserv-dev t20141205_b531 b701 b182 2015_01 v9_2-rc1 2015_02 2015_03 b376 2014_08 b801 2014_10 2014_11 2014_12 b604 b601 t20141107_b449 b449 b609 v9_1_1 b750 b751 b512 b513 b299 b598 b949 b944 b345 n20150416_b1100 HSC-latest webserv_2 webserv_1 v10_0 b427 b1062 b974 b330 b337 b539 b531 b437 v10_1_rc2 v10_1_rc3 v10_1_rc1 b759 v9_2 v9_1 b173 b176 b245 b1100 b1105 b1104 test b641 b879 v3_2_1 n20150419_b1104 qserv maf0_2 t20141208_b539 b551 b557 b991 b992 maf_0_2_2 b792 b163 b710 b658 b652 b650 b842 setup
          eigen                 3.2               HSC HSC-latest setup
          fftw                  3.3.3             sims b824 n20150327_b949 b780 b666 HSC b662 b649 2015_02 b858 qserv-dev t20141205_b531 b701 b182 2015_01 v9_2-rc1 2014_09 2015_03 b376 2014_08 b801 2014_10 2014_11 2014_12 b604 b601 t20141107_b449 b449 b609 v9_1_1 b750 b751 b512 b513 b299 b949 b944 b345 n20150416_b1100 HSC-latest webserv_2 webserv_1 v10_0 b427 b1062 b974 b330 b337 b539 b531 b437 v10_1_rc2 v10_1_rc3 v10_1_rc1 b759 v9_2 v9_1 b173 b176 b245 b710 b1100 b1105 b1104 test b641 b879 n20150419_b1104 qserv maf0_2 t20141208_b539 b551 b557 b991 b992 maf_0_2_2 b792 b163 b659 b652 b842 setup
          freetype              2.5.1             HSC current HSC-latest setup
          gcc                   4.6.4             HSC HSC-latest setup
          geom                  HSC-3.0.0d_hsc    HSC-latest setup
          gmp                   5.1.3             HSC HSC-latest setup
          gsl                   1.16              b513 b531 b512 v9_2 2014_09 v9_1 b173 b176 2014_10 2014_11 v9_1_1 t20141205_b531 b449 v9_2-rc1 b427 b182 HSC b299 b330 b337 HSC-latest t20141107_b449 b345 b437 b376 2014_08 b163 b245 setup
          hscAstrom             3.2.0f_hsc        HSC-latest setup
          hscDb                 3.7.0a_hsc        HSC-latest setup
          hscHpx                3.0.1c_hsc        HSC-latest setup
          hscPipe               3.7.1             setup
          hscPipeBase           3.3.0a_hsc        setup
          ip_diffim             HSC-3.2.0a_hsc    HSC-latest setup
          ip_isr                HSC-3.3.0a_hsc    HSC-latest setup
          libpng                1.6.7             HSC current HSC-latest setup
          matplotlib            1.3.1             HSC HSC-latest setup
          meas_algorithms       HSC-3.8.0a_hsc    setup
          meas_astrom           HSC-3.4.1         HSC-latest setup
          meas_deblender        HSC-3.6.0a_hsc    HSC-latest setup
          meas_extensions_multiShapelet HSC-3.1.2e_hsc    HSC-latest setup
          meas_extensions_photometryKron HSC-3.4.0e_hsc   HSC-latest setup
          meas_extensions_psfex HSC-3.1.1d_hsc    HSC-latest setup
          meas_extensions_rotAngle HSC-3.0.0l_hsc         HSC-latest setup
          meas_extensions_shapeHSM HSC-3.4.0a_hsc         setup
          meas_extensions_simpleShape HSC-3.0.0l_hsc      HSC-latest setup
          meas_mosaic           HSC-3.4.0a_hsc    setup
          meas_multifit         HSC-3.5.0a_hsc    setup
          minuit2               5.28.00           sims b824 b337 b780 2014_09 HSC b649 b858 qserv-dev t20141205_b531 b701 b182 2015_01 v9_2-rc1 2015_02 2015_03 b376 2014_08 b801 2014_10 2014_11 2014_12 b604 b601 t20141107_b449 b449 b609 v9_1_1 b750 b751 b512 b513 b299 b949 b944 b345 n20150416_b1100 HSC-latest webserv_2 webserv_1 v10_0 b427 b1062 b974 b330 n20150327_b949 b539 b531 b437 v10_1_rc2 v10_1_rc3 v10_1_rc1 b759 v9_2 v9_1 b173 b176 b245 b1100 b1105 b1104 test b641 b879 n20150419_b1104 qserv maf0_2 t20141208_b539 b551 b557 b991 b992 maf_0_2_2 b792 b163 b710 b652 b842 setup
          mkl                   10.3              current setup
          mpc                   1.0.1a_hsc        HSC HSC-latest setup
          mpfr                  3.1.2a_hsc        HSC HSC-latest setup
          mpi4py                1.3.1a_hsc        HSC-latest setup
          mpich                 3.1.1             HSC-latest setup
          mysqlclient           5.1.73            b531 HSC-latest b437 b163 b513 v9_2 2014_09 v9_1 HSC b176 b245 b261 2014_10 2014_11 v9_1_1 t20141205_b531 t20141107_b449 b449 v9_2-rc1 b427 b182 b173 b299 b330 b337 b345 b424 b376 2014_08 b512 setup
          ndarray               HSC-3.0.0d_hsc    HSC-latest setup
          numpy                 1.8.0             HSC HSC-latest current setup
          obs_subaru            LOCAL:/tigress/pprice/dm-2787/obs_subaru  setup
          obs_test              HSC-3.0.0e_hsc    HSC-latest setup
          pex_config            HSC-3.1.0         HSC-latest setup
          pex_exceptions        HSC-3.0.0d_hsc    HSC-latest setup
          pex_logging           HSC-3.0.0d_hsc    HSC-latest setup
          pex_policy            HSC-3.0.0d_hsc    HSC-latest setup
          pipe_base             HSC-3.2.0a_hsc    HSC-latest setup
          pipe_tasks            HSC-3.8.1a_hsc    setup
          postgresql            9.3.2             HSC HSC-latest setup
          psfex                 HSC-3.2.0b_hsc    HSC-latest setup
          psycopg2              2.5.1             HSC HSC-latest setup
          pyfits                3.2               HSC HSC-latest setup
          python                2.7.6             HSC HSC-latest current setup
          runOnsite             3.2.0a_hsc        HSC-latest setup
          scons                 2.3.0             v9_1 HSC HSC-latest b163 setup
          sconsUtils            HSC-3.1.0         HSC-latest setup
          shapelet              HSC-3.0.0l_hsc    HSC-latest setup
          skymap                HSC-3.1.0k_hsc    HSC-latest setup
          skypix                HSC-3.0.0l_hsc    HSC-latest setup
          solvetansip           6.5.1d_hsc        setup
          sqlite                3.8.2             HSC HSC-latest setup
          swig                  2.0.12            b4 b5 b6 b3 v9_2 2014_09 v9_1 b176 b245 b261 b299 v9_2-rc1 b182 b173 b330 b337 b128 v9_1_1 b345 2014_08 b163 Winter2014 HSC-latest setup
          tcltk                 8.5.15            HSC HSC-latest setup
          toolchain             LSST              current setup
          utils                 HSC-3.0.0d_hsc    HSC-latest setup
          wcslib                4.14              HSC HSC-latest setup
          xpa                   2.1.15            b531 b345 b163 b513 v9_2 2014_09 v9_1 HSC b176 b245 2014_10 2014_11 v9_1_1 t20141205_b531 b449 b427 b182 b173 b330 b337 t20141107_b449 v9_2-rc1 b437 b376 2014_08 b512 b299 HSC-latest setup
          $ hscProcessCcd.py /tigress/HSC/HSC --rerun price/dm-2787 --id visit=11690 ccd=4 
          […]
          2015-05-26T15:47:23: processCcd.calibrate.repair: Identified 337 cosmic rays.
          2015-05-26T15:49:56: processCcd.calibrate.detection: Detected 142 positive sources to 5 sigma.
          […]
          

          I am using obs_subaru HSC-3.9.2 and afw HSC-3.9.2 with 81b1645, 79337bb and 61b6195 reverted.

          Show
          price Paul Price added a comment - Maybe I got the ccd number wrong. Try visit=11690 ccd=4 : $ eups list -s afw LOCAL:/tigress/pprice/dm-2787/afw setup astrometry_net 0.50 v9_1 HSC-latest b163 setup astrometry_net_data ps1_pv2_20150302 setup base HSC-3.0.0f_hsc HSC-latest setup boost 1.55.0 b3 HSC HSC-latest setup cfitsio 3.360 HSC HSC-latest setup clapack 3.2.1 current HSC-latest setup coadd_utils HSC-3.0.0l_hsc HSC-latest setup daf_base HSC-3.1.0 HSC-latest setup daf_butlerUtils HSC-3.3.0e_hsc HSC-latest setup daf_persistence HSC-3.1.0 HSC-latest setup distEst HSC-3.0.0l_hsc HSC-latest setup doxygen 1.8.5 sims b824 n20150327_b949 b780 2014_09 HSC b660 b649 b261 b858 qserv-dev t20141205_b531 b701 b182 2015_01 v9_2-rc1 2015_02 2015_03 b376 2014_08 b801 2014_10 2014_11 2014_12 b604 b601 t20141107_b449 b449 b609 v9_1_1 b750 b751 b512 b513 b299 b598 b949 b944 b345 n20150416_b1100 HSC-latest webserv_2 webserv_1 v10_0 b427 b1062 b974 b330 b337 b539 b531 b437 v10_1_rc2 v10_1_rc3 v10_1_rc1 b759 v9_2 v9_1 b173 b176 b245 b1100 b1105 b1104 test b641 b879 v3_2_1 n20150419_b1104 qserv maf0_2 t20141208_b539 b551 b557 b991 b992 maf_0_2_2 b792 b163 b710 b658 b652 b650 b842 setup eigen 3.2 HSC HSC-latest setup fftw 3.3.3 sims b824 n20150327_b949 b780 b666 HSC b662 b649 2015_02 b858 qserv-dev t20141205_b531 b701 b182 2015_01 v9_2-rc1 2014_09 2015_03 b376 2014_08 b801 2014_10 2014_11 2014_12 b604 b601 t20141107_b449 b449 b609 v9_1_1 b750 b751 b512 b513 b299 b949 b944 b345 n20150416_b1100 HSC-latest webserv_2 webserv_1 v10_0 b427 b1062 b974 b330 b337 b539 b531 b437 v10_1_rc2 v10_1_rc3 v10_1_rc1 b759 v9_2 v9_1 b173 b176 b245 b710 b1100 b1105 b1104 test b641 b879 n20150419_b1104 qserv maf0_2 t20141208_b539 b551 b557 b991 b992 maf_0_2_2 b792 b163 b659 b652 b842 setup freetype 2.5.1 HSC current HSC-latest setup gcc 4.6.4 HSC HSC-latest setup geom HSC-3.0.0d_hsc HSC-latest setup gmp 5.1.3 HSC HSC-latest setup gsl 1.16 b513 b531 b512 v9_2 2014_09 v9_1 b173 b176 2014_10 2014_11 v9_1_1 t20141205_b531 b449 v9_2-rc1 b427 b182 HSC b299 b330 b337 HSC-latest t20141107_b449 b345 b437 b376 2014_08 b163 b245 setup hscAstrom 3.2.0f_hsc HSC-latest setup hscDb 3.7.0a_hsc HSC-latest setup hscHpx 3.0.1c_hsc HSC-latest setup hscPipe 3.7.1 setup hscPipeBase 3.3.0a_hsc setup ip_diffim HSC-3.2.0a_hsc HSC-latest setup ip_isr HSC-3.3.0a_hsc HSC-latest setup libpng 1.6.7 HSC current HSC-latest setup matplotlib 1.3.1 HSC HSC-latest setup meas_algorithms HSC-3.8.0a_hsc setup meas_astrom HSC-3.4.1 HSC-latest setup meas_deblender HSC-3.6.0a_hsc HSC-latest setup meas_extensions_multiShapelet HSC-3.1.2e_hsc HSC-latest setup meas_extensions_photometryKron HSC-3.4.0e_hsc HSC-latest setup meas_extensions_psfex HSC-3.1.1d_hsc HSC-latest setup meas_extensions_rotAngle HSC-3.0.0l_hsc HSC-latest setup meas_extensions_shapeHSM HSC-3.4.0a_hsc setup meas_extensions_simpleShape HSC-3.0.0l_hsc HSC-latest setup meas_mosaic HSC-3.4.0a_hsc setup meas_multifit HSC-3.5.0a_hsc setup minuit2 5.28.00 sims b824 b337 b780 2014_09 HSC b649 b858 qserv-dev t20141205_b531 b701 b182 2015_01 v9_2-rc1 2015_02 2015_03 b376 2014_08 b801 2014_10 2014_11 2014_12 b604 b601 t20141107_b449 b449 b609 v9_1_1 b750 b751 b512 b513 b299 b949 b944 b345 n20150416_b1100 HSC-latest webserv_2 webserv_1 v10_0 b427 b1062 b974 b330 n20150327_b949 b539 b531 b437 v10_1_rc2 v10_1_rc3 v10_1_rc1 b759 v9_2 v9_1 b173 b176 b245 b1100 b1105 b1104 test b641 b879 n20150419_b1104 qserv maf0_2 t20141208_b539 b551 b557 b991 b992 maf_0_2_2 b792 b163 b710 b652 b842 setup mkl 10.3 current setup mpc 1.0.1a_hsc HSC HSC-latest setup mpfr 3.1.2a_hsc HSC HSC-latest setup mpi4py 1.3.1a_hsc HSC-latest setup mpich 3.1.1 HSC-latest setup mysqlclient 5.1.73 b531 HSC-latest b437 b163 b513 v9_2 2014_09 v9_1 HSC b176 b245 b261 2014_10 2014_11 v9_1_1 t20141205_b531 t20141107_b449 b449 v9_2-rc1 b427 b182 b173 b299 b330 b337 b345 b424 b376 2014_08 b512 setup ndarray HSC-3.0.0d_hsc HSC-latest setup numpy 1.8.0 HSC HSC-latest current setup obs_subaru LOCAL:/tigress/pprice/dm-2787/obs_subaru setup obs_test HSC-3.0.0e_hsc HSC-latest setup pex_config HSC-3.1.0 HSC-latest setup pex_exceptions HSC-3.0.0d_hsc HSC-latest setup pex_logging HSC-3.0.0d_hsc HSC-latest setup pex_policy HSC-3.0.0d_hsc HSC-latest setup pipe_base HSC-3.2.0a_hsc HSC-latest setup pipe_tasks HSC-3.8.1a_hsc setup postgresql 9.3.2 HSC HSC-latest setup psfex HSC-3.2.0b_hsc HSC-latest setup psycopg2 2.5.1 HSC HSC-latest setup pyfits 3.2 HSC HSC-latest setup python 2.7.6 HSC HSC-latest current setup runOnsite 3.2.0a_hsc HSC-latest setup scons 2.3.0 v9_1 HSC HSC-latest b163 setup sconsUtils HSC-3.1.0 HSC-latest setup shapelet HSC-3.0.0l_hsc HSC-latest setup skymap HSC-3.1.0k_hsc HSC-latest setup skypix HSC-3.0.0l_hsc HSC-latest setup solvetansip 6.5.1d_hsc setup sqlite 3.8.2 HSC HSC-latest setup swig 2.0.12 b4 b5 b6 b3 v9_2 2014_09 v9_1 b176 b245 b261 b299 v9_2-rc1 b182 b173 b330 b337 b128 v9_1_1 b345 2014_08 b163 Winter2014 HSC-latest setup tcltk 8.5.15 HSC HSC-latest setup toolchain LSST current setup utils HSC-3.0.0d_hsc HSC-latest setup wcslib 4.14 HSC HSC-latest setup xpa 2.1.15 b531 b345 b163 b513 v9_2 2014_09 v9_1 HSC b176 b245 2014_10 2014_11 v9_1_1 t20141205_b531 b449 b427 b182 b173 b330 b337 t20141107_b449 v9_2-rc1 b437 b376 2014_08 b512 b299 HSC-latest setup $ hscProcessCcd.py /tigress/HSC/HSC --rerun price/dm-2787 --id visit=11690 ccd=4 […] 2015-05-26T15:47:23: processCcd.calibrate.repair: Identified 337 cosmic rays. 2015-05-26T15:49:56: processCcd.calibrate.detection: Detected 142 positive sources to 5 sigma. […] I am using obs_subaru HSC-3.9.2 and afw HSC-3.9.2 with 81b1645, 79337bb and 61b6195 reverted.
          Hide
          swinbank John Swinbank added a comment -

          Thanks Paul; I can replicate the problem on CCD 4 and can confirm it is addressed by the fix I referred to above. Example timings:

          Original:

          real	2m33.985s
          user	2m8.543s
          sys	0m4.429s
          

          Naive RLE:

          real	8m24.235s
          user	7m59.177s
          sys	0m3.427s
          

          Optimized RLE:

          real	2m14.081s
          user	1m50.920s
          sys	0m3.232s
          

          I'll take another look through the fix to make sure it's in a coherent state & then push it out for review.

          Show
          swinbank John Swinbank added a comment - Thanks Paul; I can replicate the problem on CCD 4 and can confirm it is addressed by the fix I referred to above. Example timings: Original: real 2m33.985s user 2m8.543s sys 0m4.429s Naive RLE: real 8m24.235s user 7m59.177s sys 0m3.427s Optimized RLE: real 2m14.081s user 1m50.920s sys 0m3.232s I'll take another look through the fix to make sure it's in a coherent state & then push it out for review.
          Hide
          price Paul Price added a comment -

          Glad we got to the bottom of this! Sorry it took longer than we would have liked...

          Show
          price Paul Price added a comment - Glad we got to the bottom of this! Sorry it took longer than we would have liked...
          Hide
          swinbank John Swinbank added a comment -

          Thanks for your help in tracking this down, Paul. Would you mind taking it on for review?

          There's just a single commit on afw's tickets/DM-2787. Since there are no functionality changes, this should already be covered by the existing unit tests.

          Show
          swinbank John Swinbank added a comment - Thanks for your help in tracking this down, Paul. Would you mind taking it on for review? There's just a single commit on afw 's tickets/DM-2787 . Since there are no functionality changes, this should already be covered by the existing unit tests.
          Hide
          price Paul Price added a comment -

          Nice code and style. Some minor comments:

          • The commit could use a more detailed commit message (e.g., what was the regression, how was it fixed?).
          • Should the iteration over foot.getSpans() and element use cbegin and cend?
          • I think xmin, xmax and yval should be const. Also start (but not end).
          • Since the Footprint so grown is normalised, if you kept track of the number of pixels and the bounding box as you went, you could set _normalized = true, which might be a boon to the user (especially since a call to getArea() immediately after calling this method would return the wrong value, since it doesn't check to see if the Footprint is normalised, and so the old area would be returned).
          Show
          price Paul Price added a comment - Nice code and style. Some minor comments: The commit could use a more detailed commit message (e.g., what was the regression, how was it fixed?). Should the iteration over foot.getSpans() and element use cbegin and cend ? I think xmin , xmax and yval should be const . Also start (but not end ). Since the Footprint so grown is normalised, if you kept track of the number of pixels and the bounding box as you went, you could set _normalized = true , which might be a boon to the user (especially since a call to getArea() immediately after calling this method would return the wrong value, since it doesn't check to see if the Footprint is normalised, and so the old area would be returned).
          Hide
          swinbank John Swinbank added a comment -

          Thanks for the speedy turnaround.

          I agree with all of your comments, except the last point re keeping track of the pixel count and bounding box. By my reading, the calls to addSpanInSeries() should automatically adjust the pixel count and bounding box as spans are added. The bounding box of the grown Footprint is already checked in tests/footprint1.py. I'll add another check to that test case to ensure the area is also correct.

          Show
          swinbank John Swinbank added a comment - Thanks for the speedy turnaround. I agree with all of your comments, except the last point re keeping track of the pixel count and bounding box. By my reading, the calls to addSpanInSeries() should automatically adjust the pixel count and bounding box as spans are added. The bounding box of the grown Footprint is already checked in tests/footprint1.py . I'll add another check to that test case to ensure the area is also correct.
          Hide
          price Paul Price added a comment -

          I think you're right.

          Show
          price Paul Price added a comment - I think you're right.

            People

            Assignee:
            swinbank John Swinbank
            Reporter:
            swinbank John Swinbank
            Reviewers:
            Paul Price
            Watchers:
            Jim Bosch, John Swinbank, Paul Price, Robert Lupton
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins Builds

                No builds found.