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

AssembleCoadd option to save image of exposures for each pixel.

    Details

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

      Description

      Create option in assembleCoadd to create and save an imageU of the number of exposures which contribute to each pixel of the final coadd.

        Attachments

          Issue Links

            Activity

            Hide
            pgee Perry Gee added a comment -

            The second question has to do with getting the after-statisticsStack count. Jim suggested on this thread that statisticsStack might return the NPOINT for each pixel, which would be the ideal output of nImage. But as far as I am concerned, the before-statisticsStack count is adequate for depth evaluation.

            The NO_DATA question was a response to the Slack message you sent me in August. Sounds like you have the problem covered.

            Show
            pgee Perry Gee added a comment - The second question has to do with getting the after-statisticsStack count. Jim suggested on this thread that statisticsStack might return the NPOINT for each pixel, which would be the ideal output of nImage. But as far as I am concerned, the before-statisticsStack count is adequate for depth evaluation. The NO_DATA question was a response to the Slack message you sent me in August. Sounds like you have the problem covered.
            Hide
            pgee Perry Gee added a comment - - edited

            Yusra AlSayyad

            I put my responses to your review in a new rebased tickets/DM-10907. Please re-review if necessary.

            I think I responded to all of your questions, and fixed the problems from by bad merger.

            I'm not sure why you were concerned about manipulating the mask array. I usually use ndarray functions to do this kind of logic, and I am not aware of lsst.afw.image.Image having these kind of abilities. I did modify the logic slightly, so it would read better. Let me know if you have a better way.

            Oh, I'm also not sure why you wanted me to look at the statistics changes. Doesn't seem to affect my code.

            Show
            pgee Perry Gee added a comment - - edited Yusra AlSayyad I put my responses to your review in a new rebased tickets/ DM-10907 . Please re-review if necessary. I think I responded to all of your questions, and fixed the problems from by bad merger. I'm not sure why you were concerned about manipulating the mask array. I usually use ndarray functions to do this kind of logic, and I am not aware of lsst.afw.image.Image having these kind of abilities. I did modify the logic slightly, so it would read better. Let me know if you have a better way. Oh, I'm also not sure why you wanted me to look at the statistics changes. Doesn't seem to affect my code.
            Hide
            yusra Yusra AlSayyad added a comment -

            Re: the statistics changes, the version of 10907 I was reviewing had some changes to SafeClipAssembleCoaddTask that collided with the statistics changes. (e.g. call signature of SafeClipAssembleCoaddTask.assemble() and the arguments passed to AssembleCoaddTask.assemble() which had also been changed on master: https://github.com/lsst/pipe_tasks/commit/80f0bf60ce45f61528303bf088e77b5b84554072#diff-f9cf63fcf2aed29eec448bae6d243b5fL548 But it looks like your rebase took care of that.

            I just merged DM-11432, and there's a new Task in there: CompareWarpAssembleCoaddTask. Looking through it, it looks like it should just work when you add this ticket, but the docstring for CompareWarpAssembleCoaddTask.assemble is now a lie, and so is the variable name that it returns coaddExp:
            https://github.com/lsst/pipe_tasks/blob/master/python/lsst/pipe/tasks/assembleCoadd.py#L1562

            You can run the new assemble Task by using the optional argument --compareWarpCoadd
            assembleCoadd.py --compareWarpCoadd input/repo --output output/repo --id ...

            Other than that, tickets/DM-10907 looks good and OK to merge.

            Show
            yusra Yusra AlSayyad added a comment - Re: the statistics changes, the version of 10907 I was reviewing had some changes to SafeClipAssembleCoaddTask that collided with the statistics changes. (e.g. call signature of SafeClipAssembleCoaddTask.assemble() and the arguments passed to AssembleCoaddTask.assemble() which had also been changed on master: https://github.com/lsst/pipe_tasks/commit/80f0bf60ce45f61528303bf088e77b5b84554072#diff-f9cf63fcf2aed29eec448bae6d243b5fL548 But it looks like your rebase took care of that. I just merged DM-11432 , and there's a new Task in there: CompareWarpAssembleCoaddTask. Looking through it, it looks like it should just work when you add this ticket, but the docstring for CompareWarpAssembleCoaddTask.assemble is now a lie, and so is the variable name that it returns coaddExp : https://github.com/lsst/pipe_tasks/blob/master/python/lsst/pipe/tasks/assembleCoadd.py#L1562 You can run the new assemble Task by using the optional argument --compareWarpCoadd assembleCoadd.py --compareWarpCoadd input/repo --output output/repo --id ... Other than that, tickets/ DM-10907 looks good and OK to merge.
            Hide
            yusra Yusra AlSayyad added a comment -

            Looks like you merged with without testing with the --compareWarp flag or reviewing the commit after merging in the changes on master:

            In particular, the merge removed the supplementaryData argument in the assemble() definition: https://github.com/lsst/pipe_tasks/commit/a8c9ca5435591c290dc1696b4f5acbb6b841bab9#diff-f9cf63fcf2aed29eec448bae6d243b5fR373

            I suspect you were only running the unit tests which only exercise the basic assemble. I'm adding the other two varieties now in DM-11933
            I'll fix this under the same ticket.

            Show
            yusra Yusra AlSayyad added a comment - Looks like you merged with without testing with the --compareWarp flag or reviewing the commit after merging in the changes on master: In particular, the merge removed the supplementaryData argument in the assemble() definition: https://github.com/lsst/pipe_tasks/commit/a8c9ca5435591c290dc1696b4f5acbb6b841bab9#diff-f9cf63fcf2aed29eec448bae6d243b5fR373 I suspect you were only running the unit tests which only exercise the basic assemble. I'm adding the other two varieties now in DM-11933 I'll fix this under the same ticket.
            Hide
            pgee Perry Gee added a comment -

            I had a lot of difficulty doing this merge, because I have not been able to build on my own machines. The lsstsw rebuild is not working right now, and my current stack seems incompatible with the last version of pipe_tasks.

            Sorry if I did any damage. I think that it was probably the right choice for me to run Jenkins successfully and then merge, but I wasn't able to easily check everything.

            Let me know what I should do now that we are in the state we are in.

            Show
            pgee Perry Gee added a comment - I had a lot of difficulty doing this merge, because I have not been able to build on my own machines. The lsstsw rebuild is not working right now, and my current stack seems incompatible with the last version of pipe_tasks. Sorry if I did any damage. I think that it was probably the right choice for me to run Jenkins successfully and then merge, but I wasn't able to easily check everything. Let me know what I should do now that we are in the state we are in.

              People

              • Assignee:
                pgee Perry Gee
                Reporter:
                pgee Perry Gee
                Reviewers:
                Yusra AlSayyad
                Watchers:
                Jim Bosch, Perry Gee, Yusra AlSayyad
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel