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

Add aperture corrections to meas_extensions_photometryKron

    Details

      Description

      When transitioning meas_extensions_photometryKron to the new measurement framework, aperture correction was omitted pending the completion of DM-85. It needs to be re-enabled when that epic is complete.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            If we do test aperture correction here at all (and I'm not saying we need to, at least on this issue), I think it's sufficient to merely test their application, not their measurement (since the latter requires doing quite a bit more and is tested elsewhere). In other words, we could simply mock up an ApCorrMap with a constant ChebyshevBoundedField and verify that running measurement with aperture correction enabled scales the expected result by the value in the ChebyshevBoundedField.

            Show
            jbosch Jim Bosch added a comment - If we do test aperture correction here at all (and I'm not saying we need to, at least on this issue), I think it's sufficient to merely test their application, not their measurement (since the latter requires doing quite a bit more and is tested elsewhere). In other words, we could simply mock up an ApCorrMap with a constant ChebyshevBoundedField and verify that running measurement with aperture correction enabled scales the expected result by the value in the ChebyshevBoundedField .
            Hide
            rowen Russell Owen added a comment -

            Jim: that is an excellent suggestion. I'll give that a try. It will add some time to the ticket, but is worth doing.

            Show
            rowen Russell Owen added a comment - Jim: that is an excellent suggestion. I'll give that a try. It will add some time to the ticket, but is worth doing.
            Hide
            rowen Russell Owen added a comment - - edited

            Lauren: do you have time to look at this?

            All the work is on meas_extensions_photometryKron tickets/DM-2429

            I was able to add a test for aperture correction to the existing unit test.

            Show
            rowen Russell Owen added a comment - - edited Lauren: do you have time to look at this? All the work is on meas_extensions_photometryKron tickets/ DM-2429 I was able to add a test for aperture correction to the existing unit test.
            Hide
            lauren Lauren MacArthur added a comment -

            It looks like you should squash these two commits:
            54d4327 2015-08-14 Add a test that aperture correction works. [Russell Owen]
            768f5a7 2015-08-14 starting to add ap corr to unit test [Russell Owen]

            Otherwise, I built your branch and ran processCcd.py on HSC data and all looks ok within the context of this ticket (per Jim's comment above).

            After squashing, feel free to merge to master.

            Show
            lauren Lauren MacArthur added a comment - It looks like you should squash these two commits: 54d4327 2015-08-14 Add a test that aperture correction works. [Russell Owen] 768f5a7 2015-08-14 starting to add ap corr to unit test [Russell Owen] Otherwise, I built your branch and ran processCcd.py on HSC data and all looks ok within the context of this ticket (per Jim's comment above). After squashing, feel free to merge to master.
            Hide
            rowen Russell Owen added a comment -

            Thank you for catching the missing squash. I did that and merged to master.

            Show
            rowen Russell Owen added a comment - Thank you for catching the missing squash. I did that and merged to master.

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                swinbank John Swinbank
                Reviewers:
                Lauren MacArthur
                Watchers:
                Jim Bosch, John Swinbank, Lauren MacArthur, Paul Price, Perry Gee, Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: