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

ImageDifferenceTask: Refactor Image DifferenceTask

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Won't Fix
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pipe_tasks
    • Labels:
      None
    • Story Points:
      8
    • Sprint:
      AP S18-6, AP F18-1, AP F18-2, AP F18-3, AP F18-4, AP F18-5
    • Team:
      Alert Production

      Description

      The original DM-3704 was to refactor all ImageDifference task. This issue was split into 3 tasks:
      1) Split image difference task into two tasks (1) to generate an image difference, and (2) to run detection and measurement on it: processDiffim.py
      2) Refactor the Image Difference portion
      3) Refactor the processDiffim portion

      This ticket refactors the new task that just generates and image difference.

        Attachments

          Issue Links

            Activity

            Hide
            reiss David Reiss added a comment - - edited

            Status: in branch u/djreiss/DM-3704 we have split imageDifference.py into two separate command-line tasks, makeDiffim.py and processDiffim.py. Those two tasks now reside in ip_diffim, and their corresponding calls from command-line are in pipe_tasks. We have also written a (temporary) imageDifference2.py task in pipe_tasks which is meant to perform identically to the original imageDifference.py but by calling the two new tasks in order, as subtasks. Config parameters have been moved into the appropriate subtasks.

            It all seems to work.

            Preliminary tests show that imageDifference2.py performs identical processing to the original script, and generates output similar to the original script in the butler repo. Similarly, the two new command-line tasks are fully functional and perform as expected. The output diaSource catalogs are indentical in size, but not bit-wise identical, however I have confirmed that they contain identical content.

            It was found that certain config parameters (debugging flags which were set to False by default) were not functional (likely bit-rotted) in the original imageDifference.py task, and they are not functional in the new imageDifference2.py. These include doAddMetrics and doMatchSources. There are other related parameters such as doSelectDcrCatalog/doSelectVariableCatalog which have not been tested as they depend upon the other two parameters being functional.

            Some refactoring of the individual makeDiffim and processDiffim tasks has been performed, including some but not all listed in DM-3704.

            Show
            reiss David Reiss added a comment - - edited Status: in branch u/djreiss/ DM-3704 we have split imageDifference.py into two separate command-line tasks, makeDiffim.py and processDiffim.py . Those two tasks now reside in ip_diffim , and their corresponding calls from command-line are in pipe_tasks . We have also written a (temporary) imageDifference2.py task in pipe_tasks which is meant to perform identically to the original imageDifference.py but by calling the two new tasks in order, as subtasks. Config parameters have been moved into the appropriate subtasks. It all seems to work. Preliminary tests show that imageDifference2.py performs identical processing to the original script, and generates output similar to the original script in the butler repo. Similarly, the two new command-line tasks are fully functional and perform as expected. The output diaSource catalogs are indentical in size, but not bit-wise identical, however I have confirmed that they contain identical content. It was found that certain config parameters (debugging flags which were set to False by default) were not functional (likely bit-rotted) in the original imageDifference.py task, and they are not functional in the new imageDifference2.py . These include doAddMetrics and doMatchSources . There are other related parameters such as doSelectDcrCatalog / doSelectVariableCatalog which have not been tested as they depend upon the other two parameters being functional. Some refactoring of the individual makeDiffim and processDiffim tasks has been performed, including some but not all listed in DM-3704 .
            Hide
            reiss David Reiss added a comment -

            I believe I have made doMatchSources functional again.

            Show
            reiss David Reiss added a comment - I believe I have made doMatchSources functional again.
            Hide
            reiss David Reiss added a comment -

            doAddMetrics error:

            makeDiffim FATAL: Failed on dataId=DataId(initialdata={'visit': 289820, 'ccdnum': 11}, tag=set([])): 'PointD'
            Traceback (most recent call last):
              File "/Users/dreiss/lsstsw/stack/DarwinX86/pipe_base/13.0-14-g8b3bf66+1/python/lsst/pipe/base/cmdLineTask.py", line 407, in __call__
                result = task.run(dataRef, **kwargs)
              File "/Users/dreiss/lsstsw/stack/DarwinX86/pipe_base/13.0-14-g8b3bf66+1/python/lsst/pipe/base/timer.py", line 150, in wrapper
                res = func(self, *args, **keyArgs)
              File "/Users/dreiss/GIT_REPOS/TICKETS/DM-3704/temp/ip_diffim/python/lsst/ip/diffim/makeDiffim.py", line 263, in run
                result = self.doMakeDiffim(template, exposure, selectSources, idFactory=idFactory)
              File "/Users/dreiss/lsstsw/stack/DarwinX86/pipe_base/13.0-14-g8b3bf66+1/python/lsst/pipe/base/timer.py", line 150, in wrapper
                res = func(self, *args, **keyArgs)
              File "/Users/dreiss/GIT_REPOS/TICKETS/DM-3704/temp/ip_diffim/python/lsst/ip/diffim/makeDiffim.py", line 326, in doMakeDiffim
                selectSources, idFactory)
              File "/Users/dreiss/GIT_REPOS/TICKETS/DM-3704/temp/ip_diffim/python/lsst/ip/diffim/makeDiffim.py", line 404, in runSelectSources
                kcQa = KernelCandidateQa(nparam)
              File "/Users/dreiss/GIT_REPOS/TICKETS/DM-3704/temp/ip_diffim/python/lsst/ip/diffim/kernelCandidateQa.py", line 50, in __init__
                self.fields.append(afwTable.Field["PointD"]("RegisterRefPosition",
              File "/Users/dreiss/lsstsw/stack/DarwinX86/utils/13.0-9-gf29e843+1/python/lsst/utils/wrappers.py", line 409, in __getitem__
                return self._registry[key]
            KeyError: 'PointD'
            

            Show
            reiss David Reiss added a comment - doAddMetrics error: makeDiffim FATAL: Failed on dataId=DataId(initialdata={'visit': 289820, 'ccdnum': 11}, tag=set([])): 'PointD' Traceback (most recent call last): File "/Users/dreiss/lsstsw/stack/DarwinX86/pipe_base/13.0-14-g8b3bf66+1/python/lsst/pipe/base/cmdLineTask.py", line 407, in __call__ result = task.run(dataRef, **kwargs) File "/Users/dreiss/lsstsw/stack/DarwinX86/pipe_base/13.0-14-g8b3bf66+1/python/lsst/pipe/base/timer.py", line 150, in wrapper res = func(self, *args, **keyArgs) File "/Users/dreiss/GIT_REPOS/TICKETS/DM-3704/temp/ip_diffim/python/lsst/ip/diffim/makeDiffim.py", line 263, in run result = self.doMakeDiffim(template, exposure, selectSources, idFactory=idFactory) File "/Users/dreiss/lsstsw/stack/DarwinX86/pipe_base/13.0-14-g8b3bf66+1/python/lsst/pipe/base/timer.py", line 150, in wrapper res = func(self, *args, **keyArgs) File "/Users/dreiss/GIT_REPOS/TICKETS/DM-3704/temp/ip_diffim/python/lsst/ip/diffim/makeDiffim.py", line 326, in doMakeDiffim selectSources, idFactory) File "/Users/dreiss/GIT_REPOS/TICKETS/DM-3704/temp/ip_diffim/python/lsst/ip/diffim/makeDiffim.py", line 404, in runSelectSources kcQa = KernelCandidateQa(nparam) File "/Users/dreiss/GIT_REPOS/TICKETS/DM-3704/temp/ip_diffim/python/lsst/ip/diffim/kernelCandidateQa.py", line 50, in __init__ self.fields.append(afwTable.Field["PointD"]("RegisterRefPosition", File "/Users/dreiss/lsstsw/stack/DarwinX86/utils/13.0-9-gf29e843+1/python/lsst/utils/wrappers.py", line 409, in __getitem__ return self._registry[key] KeyError: 'PointD'
            Hide
            reiss David Reiss added a comment - - edited

            The metrics part of the code is still a mess, mostly because it relies on outputs from both tasks (make/processDiffim), and each task creates and/or updates data from the other. Thus they must be generated or updated by each task and then passed back up to the calling imageDifference2.py script.

            Could use some help on ideas of how to refactor this bit.

            Show
            reiss David Reiss added a comment - - edited The metrics part of the code is still a mess, mostly because it relies on outputs from both tasks (make/processDiffim), and each task creates and/or updates data from the other. Thus they must be generated or updated by each task and then passed back up to the calling imageDifference2.py script. Could use some help on ideas of how to refactor this bit.
            Hide
            price Paul Price added a comment -

            David Reiss: user branches should be u/<username>/whatever (note the leading u/).. This is important because we all share the branch namespace. Could you please rename your branch (and delete the non-conforming one)?

            Show
            price Paul Price added a comment - David Reiss : user branches should be u/<username>/whatever (note the leading u/ ).. This is important because we all share the branch namespace. Could you please rename your branch (and delete the non-conforming one)?
            Hide
            reiss David Reiss added a comment -

            Nothing to review here, all changes are part of DM-3704.

            Show
            reiss David Reiss added a comment - Nothing to review here, all changes are part of DM-3704 .
            Hide
            swinbank John Swinbank added a comment -

            This work will be taken into account in the new design being developed on DM-21304; it won't be completed/merged as is.

            Show
            swinbank John Swinbank added a comment - This work will be taken into account in the new design being developed on DM-21304 ; it won't be completed/merged as is.

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              yusra Yusra AlSayyad
              Reviewers:
              John Parejko
              Watchers:
              David Reiss, John Parejko, John Swinbank, Paul Price, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: