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

Add aperture corrections to meas_extensions_photometryKron

    XMLWordPrintable

    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

          blocks

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-3440 add meas_extensions_photometryKron to lsstsw, lsst_distrib

          • Done
          is blocked by

          Epic - Created by Jira Software - do not edit or delete. Issue type for a big user story that needs to be broken down. DM-85 Measurement - Aperture Corrections

          • Done
          relates to

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-1953 Post meas_base move changes to Kron

          • Done

            Activity

            Ascending order - Click to sort in descending order
            16 older comments
            Hide
            Permalink
            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
            Permalink
            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
            Permalink
            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
            Permalink
            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
            Permalink
            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:

                  Jenkins

                  No builds found.