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

apply aperture corrections in measurement tasks

    Details

      Description

      We need to interpolate the aperture correction to the position of every source, and apply this correction to all appropriate fluxes.

      The HSC implementation of this work (as well as that of DM-435) was done on issue HSC-191:
      https://hsc-jira.astro.princeton.edu/jira/browse/HSC-191
      There were changes to many packages, but the relevant ones for LSST are:
      https://github.com/HyperSuprime-Cam/afw/commit/057fb3c0581c512d5664f1883a72da950c9eae9d
      https://github.com/HyperSuprime-Cam/meas_algorithms/compare/HSC-3.0.0...u/jbosch/DM-191
      https://github.com/HyperSuprime-Cam/pipe_tasks/compare/4c3a53e7238cbe9...u/jbosch/DM-191

      Note that on the LSST side, we'll want to apply the aperture corrections either within a new plugin in meas_base or as a new part of BaseMeasurementTask, not as a change to the CorrectFluxes algorithm (which will be removed in the future along with the rest of the old measurement framework in meas_algorithms).

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment - - edited

            Mario suggested, and Jim agreed, that we handle measureApCorr.py by bringing it over unchanged as a commit on tickets/DM-436 with the next commit performing minimal cleanup: just like the standard procedure, but without the sub-branch named tickets/DM-436-transfer. This makes it easier to incorporate the updated measureApCorr.py into the existing ticket (adding a sub-branch while rebasing appears to be difficult).

            I did that and applied all the other changes Jim suggested and he has agreed to have another look before merging.

            We are intentionally not enabling aperture correction in the integration test on this ticket; see DM-3114 for that.

            Show
            rowen Russell Owen added a comment - - edited Mario suggested, and Jim agreed, that we handle measureApCorr.py by bringing it over unchanged as a commit on tickets/ DM-436 with the next commit performing minimal cleanup: just like the standard procedure, but without the sub-branch named tickets/ DM-436 -transfer. This makes it easier to incorporate the updated measureApCorr.py into the existing ticket (adding a sub-branch while rebasing appears to be difficult). I did that and applied all the other changes Jim suggested and he has agreed to have another look before merging. We are intentionally not enabling aperture correction in the integration test on this ticket; see DM-3114 for that.
            Hide
            jbosch Jim Bosch added a comment -

            Looks good. There are a few more very minor comments on the meas_base PR.

            I also don't see changes in obs_sdss, which would be necessary to upgrade SdssCalibrateTask to use the new aperture correction code (which is a prerequisite for getting this running in the lsst_dm_stack_demo integration test). While I'm okay with putting that off to another issue, I think it should be a separate issue from DM-2954, and probably something worth putting as a slightly higher priority than DM-833, just because it will give us at least some test coverage of these changes.

            Speaking of test coverage, while I'm okay with moving the unit tests for this code to DM-2954, I'd like to make sure we've at least tried running it all in a one-off sense before merging it. Are you in a position to just try running processCcd.py on CFHT data on this ticket branch to verify that it doesn't blow up and that the resultant fluxes are not crazy?

            Show
            jbosch Jim Bosch added a comment - Looks good. There are a few more very minor comments on the meas_base PR. I also don't see changes in obs_sdss , which would be necessary to upgrade SdssCalibrateTask to use the new aperture correction code (which is a prerequisite for getting this running in the lsst_dm_stack_demo integration test). While I'm okay with putting that off to another issue, I think it should be a separate issue from DM-2954 , and probably something worth putting as a slightly higher priority than DM-833 , just because it will give us at least some test coverage of these changes. Speaking of test coverage, while I'm okay with moving the unit tests for this code to DM-2954 , I'd like to make sure we've at least tried running it all in a one-off sense before merging it. Are you in a position to just try running processCcd.py on CFHT data on this ticket branch to verify that it doesn't blow up and that the resultant fluxes are not crazy?
            Hide
            rowen Russell Owen added a comment - - edited

            The work on obs_sdss will be done on DM-3114 at your earlier suggestion (you did not want to bog down this ticket further).

            I have updated obs_cfht tickets/DM-436 to add aperture correction to its overridden version of the calibration task (and also brought it up to parity with the current calibration task as an initial commit). Dominique Boutigny has agreed to review that change.

            I have run DM-436 on CFHT data with aperture correction enabled and am trying to sanity check the results.

            Show
            rowen Russell Owen added a comment - - edited The work on obs_sdss will be done on DM-3114 at your earlier suggestion (you did not want to bog down this ticket further). I have updated obs_cfht tickets/ DM-436 to add aperture correction to its overridden version of the calibration task (and also brought it up to parity with the current calibration task as an initial commit). Dominique Boutigny has agreed to review that change. I have run DM-436 on CFHT data with aperture correction enabled and am trying to sanity check the results.
            Hide
            boutigny Dominique Boutigny added a comment -

            I reviewed the code in obs_cfht and do not see any issues. This part is ok to merge.

            Show
            boutigny Dominique Boutigny added a comment - I reviewed the code in obs_cfht and do not see any issues. This part is ok to merge.
            Hide
            rowen Russell Owen added a comment - - edited

            At Jim's suggestion I updated the way fluxSigma is aperture-corrected to use the naive algorithm that HSC switched to, because the earlier HSC code I brought over double-counts photon noise, potentially leading to unrealistically large values for fluxSigma.

            I also added code to report statistics on the actual affect aperture correction has on the various flux fields being corrected. This code is only run if the log level is at least DEBUG. See below for an example.

            Using a reasonable radius for the reference flux (see DM-3160) the results with CFHT are at least sane, as shown here (the log output mentioned above) for visit=850587 ccd=21 of Dominique Boutigny's CFHT demo:

            processCcd.calibrate.applyApCorr DEBUG: For flux field 'base_PsfFlux': mean apCorr=0.909917255102, stdDev apCorr=0.0169003769957, mean apCorrSigma=0.0, stdDev apCorrSigma=0.0 for 199 sources
            processCcd.calibrate.applyApCorr DEBUG: For flux field 'base_GaussianFlux': mean apCorr=1.18189568415, stdDev apCorr=0.0222403718361, mean apCorrSigma=0.0, stdDev apCorrSigma=0.0 for 199 sources
            

            Show
            rowen Russell Owen added a comment - - edited At Jim's suggestion I updated the way fluxSigma is aperture-corrected to use the naive algorithm that HSC switched to, because the earlier HSC code I brought over double-counts photon noise, potentially leading to unrealistically large values for fluxSigma. I also added code to report statistics on the actual affect aperture correction has on the various flux fields being corrected. This code is only run if the log level is at least DEBUG. See below for an example. Using a reasonable radius for the reference flux (see DM-3160 ) the results with CFHT are at least sane, as shown here (the log output mentioned above) for visit=850587 ccd=21 of Dominique Boutigny's CFHT demo: processCcd.calibrate.applyApCorr DEBUG: For flux field 'base_PsfFlux': mean apCorr=0.909917255102, stdDev apCorr=0.0169003769957, mean apCorrSigma=0.0, stdDev apCorrSigma=0.0 for 199 sources processCcd.calibrate.applyApCorr DEBUG: For flux field 'base_GaussianFlux': mean apCorr=1.18189568415, stdDev apCorr=0.0222403718361, mean apCorrSigma=0.0, stdDev apCorrSigma=0.0 for 199 sources

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                jbosch Jim Bosch
                Reviewers:
                Jim Bosch
                Watchers:
                Dominique Boutigny, Jim Bosch, John Swinbank, Lauren MacArthur, Nate Lust, Russell Owen, Simon Krughoff
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel