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

implement coaddition for aperture corrections

    XMLWordPrintable

    Details

      Attachments

        Issue Links

          Activity

          No builds found.
          jbosch Jim Bosch created issue -
          jbosch Jim Bosch made changes -
          Field Original Value New Value
          Epic Link DM-85 [ 11339 ]
          jbosch Jim Bosch made changes -
          Rank Ranked higher
          jbosch Jim Bosch made changes -
          Story Points 2
          jbosch Jim Bosch made changes -
          Link This issue has to be done before DM-834 [ DM-834 ]
          jbosch Jim Bosch made changes -
          Rank Ranked higher
          jbosch Jim Bosch made changes -
          Labels Measurement AppsPlanning Measurement
          jbosch Jim Bosch made changes -
          Team Camera Geom [ 10209 ]
          krughoff Simon Krughoff made changes -
          Story Points 2 3
          jbosch Jim Bosch made changes -
          Team Camera Geom [ 10209 ] UW [ 10300 ]
          Description We need to be able to coadd aperture corrections in much the same way we coadd PSFs.

          See the HSC-side DM-798 implementation for a prototype.
          We need to be able to coadd aperture corrections in much the same way we coadd PSFs.

          See the HSC-side HSC-798 and HSC-897 implementation for a prototype:
          https://hsc-jira.astro.princeton.edu/jira/browse/HSC-798
          https://hsc-jira.astro.princeton.edu/jira/browse/HSC-897
          with code here:
          https://github.com/HyperSuprime-Cam/meas_algorithms/compare/d2782da175c...u/jbosch/DM-798
          https://github.com/HyperSuprime-Cam/meas_algorithms/compare/c4fcab3251...u/price/HSC-897a
          https://github.com/HyperSuprime-Cam/pipe_tasks/compare/6eb48e90be12d...u/price/HSC-897a
          jbosch Jim Bosch made changes -
          Labels AppsPlanning Measurement SciencePipelines
          swinbank John Swinbank made changes -
          Assignee Jim Bosch [ jbosch ]
          rowen Russell Owen made changes -
          Assignee Russell Owen [ rowen ]
          jbosch Jim Bosch made changes -
          Labels SciencePipelines HSC SciencePipelines
          jbosch Jim Bosch made changes -
          Link This issue is FF-depended by DM-2981 [ DM-2981 ]
          krughoff Simon Krughoff made changes -
          Link This issue is contained by DLP-287 [ DLP-287 ]
          krughoff Simon Krughoff made changes -
          Link This issue is contained by DLP-287 [ DLP-287 ]
          rowen Russell Owen made changes -
          Sprint Science Pipelines DM-S15-5 [ 162 ]
          rowen Russell Owen made changes -
          Watchers Jim Bosch [ Jim Bosch ] Jim Bosch, Paul Price, Simon Krughoff [ Jim Bosch, Paul Price, Simon Krughoff ]
          Hide
          rowen Russell Owen added a comment -

          Does anyone have strong opinions on whether CoaddBoundedField should go in meas_algorithms or meas_base (or elsewhere)?

          I plan to put makeCoaddBoundedField in the same package as CoaddBoundedField, since it uses that class and ApCorrMap and not much else.

          Show
          rowen Russell Owen added a comment - Does anyone have strong opinions on whether CoaddBoundedField should go in meas_algorithms or meas_base (or elsewhere)? I plan to put makeCoaddBoundedField in the same package as CoaddBoundedField, since it uses that class and ApCorrMap and not much else.
          Hide
          jbosch Jim Bosch added a comment -

          meas_algorithms, I think, so it sits alongside CoaddPsf.

          Show
          jbosch Jim Bosch added a comment - meas_algorithms, I think, so it sits alongside CoaddPsf.
          Hide
          swinbank John Swinbank added a comment -

          Note that we should also bring across HSC-975 (or equivalent functionality) in this story – that was originally slated for merge in DM-2981, but didn't make it there.

          Show
          swinbank John Swinbank added a comment - Note that we should also bring across HSC-975 (or equivalent functionality) in this story – that was originally slated for merge in DM-2981 , but didn't make it there.
          swinbank John Swinbank made changes -
          Link This issue relates to DM-2980 [ DM-2980 ]
          swinbank John Swinbank made changes -
          Sprint Science Pipelines DM-S15-5 [ 162 ] Science Pipelines DM-S15-5, Science Pipelines DM-S15-6 [ 162, 165 ]
          swinbank John Swinbank made changes -
          Rank Ranked higher
          rowen Russell Owen made changes -
          Status To Do [ 10001 ] In Progress [ 3 ]
          Hide
          rowen Russell Owen added a comment - - edited

          meas_algorithms tickets/DM-833

          I cherry-picked the commits for:

          The oldest commit had some conflicts. Most were due to relying on things that had moved to meas_base, but one was due to lack of an object named BinnedWcs on the LSST side. This sounds like something LSST may want eventually, but which is not needed for this ticket and was added before the oldest commit above.

          pipe_tasks tickets/DM-833

          I cherry-picked commits for the following, which apply aperture correction on coadds:

          There were a few minor conflicts on inputRecorder.py: don't use afwImage.PARENT in calls to getBBox(), don't use getSerial() on the results of getId() and don't import math (since it's not used).

          Show
          rowen Russell Owen added a comment - - edited meas_algorithms tickets/ DM-833 I cherry-picked the commits for: https://hsc-jira.astro.princeton.edu/jira/browse/HSC-975 https://hsc-jira.astro.princeton.edu/jira/browse/HSC-798 https://hsc-jira.astro.princeton.edu/jira/browse/HSC-897 which add CoaddBoundedField and CoaddApCorrMap. git cherry-pick 7cbd8df1c9 68cffd3ba9 a9ac67d006 a6ae568f537d fd7bae622f df26ee217c I then squashed 6b5c958731 (a small fix to CoaddBoundedField.i) into the second of these to allow a the unit test to pass. The oldest commit had some conflicts. Most were due to relying on things that had moved to meas_base, but one was due to lack of an object named BinnedWcs on the LSST side. This sounds like something LSST may want eventually, but which is not needed for this ticket and was added before the oldest commit above. pipe_tasks tickets/ DM-833 I cherry-picked commits for the following, which apply aperture correction on coadds: https://hsc-jira.astro.princeton.edu/jira/browse/HSC-897 https://hsc-jira.astro.princeton.edu/jira/browse/HSC-1138 d5c2356, which apparently has no ticket branch in git but may be part of https://hsc-jira.astro.princeton.edu/jira/browse/HSC-976 git cherry-pick 7656d164 a0812f2f d5c2356d ba18e527 There were a few minor conflicts on inputRecorder.py: don't use afwImage.PARENT in calls to getBBox(), don't use getSerial() on the results of getId() and don't import math (since it's not used).
          Hide
          rowen Russell Owen added a comment -

          Bob Armstrong has agreed to review these changes, but he does not yet have a JIRA account, so I am leaving this in state "In Progress" for now.

          Show
          rowen Russell Owen added a comment - Bob Armstrong has agreed to review these changes, but he does not yet have a JIRA account, so I am leaving this in state "In Progress" for now.
          Hide
          rowen Russell Owen added a comment - - edited

          Since Bob Armstrong still has not been given a JIRA account he kindly emailed his review to me. Here it is:

          I went through the code and I think you ported over all the necessary changes that I could see. Since these are mostly ports straight from the HSC side I don't have any comments. Have you been able to run the code on any real HSC data? My one concern is that there is a piece that may be missing that is not fully captured by the unit tests. Unfortunately, running through real HSC coadds has been the only thing that gives me confidence that things are working as they should. Hopefully, we can work on changing that for the future. Otherwise, I think your changes are good to go.

          Show
          rowen Russell Owen added a comment - - edited Since Bob Armstrong still has not been given a JIRA account he kindly emailed his review to me. Here it is: I went through the code and I think you ported over all the necessary changes that I could see. Since these are mostly ports straight from the HSC side I don't have any comments. Have you been able to run the code on any real HSC data? My one concern is that there is a piece that may be missing that is not fully captured by the unit tests. Unfortunately, running through real HSC coadds has been the only thing that gives me confidence that things are working as they should. Hopefully, we can work on changing that for the future. Otherwise, I think your changes are good to go.
          rowen Russell Owen made changes -
          Component/s meas_algorithms [ 10732 ]
          rowen Russell Owen made changes -
          Component/s meas_base [ 10750 ]
          rowen Russell Owen made changes -
          Component/s pipe_tasks [ 10726 ]
          rowen Russell Owen made changes -
          Status In Progress [ 3 ] Reviewed [ 10101 ]
          Hide
          rowen Russell Owen added a comment -

          I tested the changes using some HSC data and it all seems to run fine. Merged to master.

          Show
          rowen Russell Owen added a comment - I tested the changes using some HSC data and it all seems to run fine. Merged to master.
          rowen Russell Owen made changes -
          Resolution Done [ 10000 ]
          Status Reviewed [ 10101 ] Done [ 10002 ]
          swinbank John Swinbank made changes -
          Labels HSC SciencePipelines SciencePipelines

            People

            Assignee:
            rowen Russell Owen
            Reporter:
            jbosch Jim Bosch
            Watchers:
            Jim Bosch, John Swinbank, Paul Price, Russell Owen, Simon Krughoff
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.