apply aperture corrections in measurement tasks

XMLWordPrintable

Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
7
• Sprint:
Science Pipelines DM-S15-4, Science Pipelines DM-S15-5
• Team:

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

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).

Activity

Hide
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
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
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
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
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
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
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
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
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
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:
Russell Owen
Reporter:
Jim Bosch
Reviewers:
Jim Bosch
Watchers:
Dominique Boutigny, Jim Bosch, John Swinbank, Lauren MacArthur, Nate Lust, Russell Owen, Simon Krughoff