# Footprint dilation performance regression

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
5
• Sprint:
Science Pipelines DM-S15-3
• Team:
Data Release Production

#### 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.

#### Activity

No builds found.
John Swinbank created issue -
Field Original Value New Value
Epic Link DM-1910 [ 15942 ]
 Watchers John Swinbank [ John Swinbank ] Jim Bosch, John Swinbank, Paul Price, Robert Lupton [ Jim Bosch, John Swinbank, Paul Price, Robert Lupton ]
Hide
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
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
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 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
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
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
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
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
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
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
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
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
Paul Price added a comment -

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

Show
Paul Price added a comment - Glad we got to the bottom of this! Sorry it took longer than we would have liked...
Hide
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
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.
 Reviewers Paul Price [ price ] Status To Do [ 10001 ] In Review [ 10004 ]
 Story Points 4 5
Hide
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
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).
 Status In Review [ 10004 ] Reviewed [ 10101 ]
Hide
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
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
Paul Price added a comment -

I think you're right.

Show
Paul Price added a comment - I think you're right.
 Resolution Done [ 10000 ] Status Reviewed [ 10101 ] Done [ 10002 ]

#### People

Assignee:
John Swinbank
Reporter:
John Swinbank
Reviewers:
Paul Price
Watchers:
Jim Bosch, John Swinbank, Paul Price, Robert Lupton