Comments:

To goal of this ticket was to get the untested imageDifferenceTask running again. This ticket was tested on decam HiTs data and Stripe 82. This ticket is an intermediate step before the following work:

• Image Difference Task overhaul. DM-3704
  • Refactor
  • Add unit tests
  • Test and include support in additional obs packages
• Dipole Measurement Task: DM-3515 (The dependency was switched, as I got this running with the old DipoleMeasurement with a few minimal changes.)
• Fix Register Task. (Need new ticket). It consistently ruins the registration and is turned off by default in this ticket.

Russell, do you have time to review? Changes are on branch tickets/DM-3213 (NOTE: ignore the user branches as they present a different solution)

obs_decam yusra$ git diff master.. --stat

 policy/DecamMapper.paf | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

 1 file changed, 205 insertions(+)

obs_sdss yusra$ git diff master.. --stat

 policy/SdssMapper.paf | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

 1 file changed, 112 insertions(+)

pipe_tasks yusra$ git diff master.. --stat

 python/lsst/pipe/tasks/diffimGetTemplate.py | 174 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

 python/lsst/pipe/tasks/imageDifference.py   | 200 ++++++++++++++++++++++++++++---------------------------------------------------------------

 2 files changed, 236 insertions(+), 138 deletions(-)

ip_diffim yusra$ git diff master.. --stat

 python/lsst/ip/diffim/diaCatalogSourceSelector.py | 4 ++--

 python/lsst/ip/diffim/diffimTools.py              | 5 ++++-

 python/lsst/ip/diffim/dipoleMeasurement.py        | 9 ++++++---

 python/lsst/ip/diffim/psfMatch.py                 | 2 +-

 4 files changed, 13 insertions(+), 7 deletions(-)

Given that this is an intermediate step, some suggestions I will delay to DM-3704. The ticket page has a running to do list.

Aside from bug fixes, there was one substantive change in this ticket: Moving getTemplate to its own subtask. I would appreciate your advice in particular on
1) the task names:
module: diffimGetTemplate.py

• GetTemplateCoaddTask (Default coadd templates)
• GetTemplateSameCcdCalexpTask
  • Shared between Winter2013ImageDifferenceTask and obs_decam processing of HiTs data
  • visit is giving in the config. gets same ccdnum as the the science image

2) How the configs are passed betweenthe subtask and ImageDifferenceTask.

• Right now they are manually set which seems clunky:

  self.makeSubtask("getTemplate")

  if self.config.getTemplate == "GetTemplateSameCcdCalexpTask":

  self.getTemplate.config.doAddCalexpBackground = self.config.doAddCalexpBackground

• In the refactor we also want the names of the image differences to depend on the types of templates used. Now we have deepDiff and goodSeeingDiff, but arguably the diffs with calexp templates should be called just diff, or singleFrameDiff. I'm thinking baout where this should be controlled from.

To test on decam HiTs data (wherein each visit is aligned – like the Winter2013 DC):

setup config file like:

$ cat diffimconfig.py

from lsst.pipe.tasks.diffimGetTemplate import GetTemplateSameCcdCalexpTask

config.getTemplate.retarget(GetTemplateSameCcdCalexpTask)

config.getTemplate.visit = 288976 #or any other visit with calexps

Run like:

$PIPE_TASKS_DIR/bin/imageDifference.py /lsst8/image_diff/calexp_dir_Blind14A_10  --id visit=289820 ccdnum=10 --configfile diffimconfig.py --output /path/to/output --clobber-config

As of writing calexps for ccdnum=1..30 available.

To test on Stripe 82 data:

$PIPE_TASKS_DIR/bin/imageDifference.py  /lsst8/yusra/RX2129_v11/calexp_dir/  --id camcol=4 field=105 filter=r run=4192  --config doSelectSources=False convolveTemplate=False --output /lsst8/yusra/RX2129_v11/coadd_r_dir  --clobber-config

I'm guessing this is just a temporary workaround, but I don't like having to specify a template visit in the configuration, because the configuration then has to change from pointing to pointing. We've done workarounds in a similar way for HSC in the past, and found it was not good. It's better to supply things like this via a command-line argument or some other mechanism.

Thank you Russell for the all suggestions on the getTemplate subtask and instructions on how to specify the template visit on the command line. One additional decision was what to do if more than one template visit number is specified: with this implementation GetCalexpAsTemplate.run() warns that it will use the first visit specified. Like we talked about, I changed the names and moved getTemplate.py to ip_diffim. Re: configs, I changed my mind on the attempt to tether doAddCalexpBackground in ImageDifference and getTemplate. It's conceivable that a user would intentionally want to change one without the other.

Now you would test on HiTs data like:

$PIPE_TASKS_DIR/bin/imageDifference.py /lsst8/image_diff/calexp_dir_Blind14A_10  --id visit=289820 ccdnum=10   --templateId visit=288976 --configfile diffimconfig.py --output /path/to/output --clobber-config

All changes are in tickets/DM-3213 in pipe_tasks, ip_diffim, obs_sdss, and obs_decam. It's ready for review when you get a chance. It'd be great if it were available when the bootcamp attendees rebuild their master stacks next Monday.

This adds some very nice improvements and clarifications. I just have a few minor comments and a request for more documentation.

pipe_tasks

imageDifference.py

• afwImage is imported but not used
• GetCoaddAsTemplateTask, GetCalexpAsTemplateTask can be imported from lsst.im.diffim; you don't need to go down into getTemplate to get to them, and doing so makes the code a bit less clear
• I suggest moving "import lsst.ip.diffim.utils as diUtils" to the top; the most important reason is to catch errors that prevent the module from being imported early, but it also reduces repetition and makes dependencies more obvious

ip_diffIm

getTemplate.py:

GetTemplateCoaddTask and GetCalexpAsTemplateTask

• These tasks are missing the doxygen comments that makes them show up on the main task documentation page. Please add that to make the tasks visible.
• The main doc strings are too brief; please fill these out as per our standard for tasks (including a brief example of how to run the task). If you are out of time for that now, please file a ticket so it does not get forgotten.

I suggest adding the configs to _all_ (in the past I have argued that it's just as good or better to get to the configs via the task's ConfigureClass method, but others disagree, and it certainly is true that it can be convenient to get to the configs directly).

imagePsfMatch.py:

ImagePsfMatchTask.makeCandidateList argument "candidateList": please document the two accepted data types in the @param description. When you refactor this code I hope you'll find you can require one of the two data types and make the user convert the other one, as it would simplify the API and the code. Also, with any luck you can clarify a function called makeCandidateList that take one kind of candidate list and return another – I find that really confusing, but not worth fixing on this ticket.

psfMatch.py

• pex.exceptions is imported but never used

Thanks for the review.
Work to be continued...

• The doxygen task documentation for imageDifference and its subtasks will be part of DM-3704, so that all the diffIm tasks will hit the main task page at once.
• Cleaning up dipole centroids and classification in DipoleMeasurementTask is next: DM-3515