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

Activate CModel prior fix

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: obs_subaru
    • Labels:
    • Team:
      Data Release Production

      Description

      DM-9795 added a fix for a CModel prior bug, but left the fix deactivated pending verification on a larger amount of data. Once verification is complete, activate the fix permanently.

      Here's the code to activate the fix, from Jim Bosch, and relevant for the CmdLineTask scripts or the multibandDriver:

      def resetCModelWeights(cfg):
          cfg.measurement.plugins["modelfit_CModel"].initial.weightsMultiplier = 1.
          cfg.measurement.plugins["modelfit_CModel"].exp.weightsMultiplier = 1.0
          cfg.measurement.plugins["modelfit_CModel"].dev.weightsMultiplier = 1.0
       
      try:
          resetCModelWeights(config)
      except AttributeError:
          resetCModelWeights(config.measureCoaddSources)
          resetCModelWeights(config.forcedPhotCoadd)
      

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            Jim Bosch, could you please confirm that we are ready to merge this?

            price@pap-laptop:~/LSST/obs_subaru (tickets/DM-10451=) $ git sub-patch
            commit 46941e2e7db1ea3f765ab9b15862d558077a9ec5
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed May 10 11:40:51 2017 -0400
             
                config: disable old behaviour of CModel
                
                DM-9795, fixed a bug in the relative weight of likelihood vs prior,
                but configuration parameters were added here to retain (approximately)
                the old behaviour. We now believe that the new behavior is far superior
                to the old and doesn't appear to introduce any ill effects, so the
                configuration changes have been removed to allow default use of the
                fixed CModel.
             
            diff --git a/config/cmodel.py b/config/cmodel.py
            index 3f13dd0..03951a9 100644
            --- a/config/cmodel.py
            +++ b/config/cmodel.py
            @@ -7,19 +7,5 @@ try:
                 config.measurement.plugins.names |= ["modelfit_DoubleShapeletPsfApprox", "modelfit_CModel"]
                 config.measurement.slots.modelFlux = 'modelfit_CModel'
                 config.catalogCalculation.plugins['base_ClassificationExtendedness'].fluxRatio = 0.985
            -
            -    # DM-9795 fixes a bug in the relative weight of likelihood vs. prior, but
            -    # before it's fully vetted we explicitly scale the likelihood below to
            -    # retain approximately the same behavior.
            -    # The median per-pixel variance in HSC Wide coadds is 0.004, and the
            -    # weights are the inverse of the sqrt of the per-pixel variance, so
            -    # multiplying the likelihood by the sqrt of the variance should
            -    # approximately cancel out the weights on average (and unit weight was the
            -    # old buggy behavior).
            -    avgPixelSigma = 0.004**0.5
            -    config.measurement.plugins["modelfit_CModel"].initial.weightsMultiplier = avgPixelSigma
            -    config.measurement.plugins["modelfit_CModel"].exp.weightsMultiplier = avgPixelSigma
            -    config.measurement.plugins["modelfit_CModel"].dev.weightsMultiplier = avgPixelSigma
            -
             except (KeyError, ImportError):
                 print("Cannot import lsst.meas.modelfit: disabling CModel measurements")
            

            Show
            price Paul Price added a comment - Jim Bosch , could you please confirm that we are ready to merge this? price@pap-laptop:~/LSST/obs_subaru (tickets/DM-10451=) $ git sub-patch commit 46941e2e7db1ea3f765ab9b15862d558077a9ec5 Author: Paul Price <price@astro.princeton.edu> Date: Wed May 10 11:40:51 2017 -0400   config: disable old behaviour of CModel DM-9795, fixed a bug in the relative weight of likelihood vs prior, but configuration parameters were added here to retain (approximately) the old behaviour. We now believe that the new behavior is far superior to the old and doesn't appear to introduce any ill effects, so the configuration changes have been removed to allow default use of the fixed CModel.   diff --git a/config/cmodel.py b/config/cmodel.py index 3f13dd0..03951a9 100644 --- a/config/cmodel.py +++ b/config/cmodel.py @@ -7,19 +7,5 @@ try: config.measurement.plugins.names |= ["modelfit_DoubleShapeletPsfApprox", "modelfit_CModel"] config.measurement.slots.modelFlux = 'modelfit_CModel' config.catalogCalculation.plugins['base_ClassificationExtendedness'].fluxRatio = 0.985 - - # DM-9795 fixes a bug in the relative weight of likelihood vs. prior, but - # before it's fully vetted we explicitly scale the likelihood below to - # retain approximately the same behavior. - # The median per-pixel variance in HSC Wide coadds is 0.004, and the - # weights are the inverse of the sqrt of the per-pixel variance, so - # multiplying the likelihood by the sqrt of the variance should - # approximately cancel out the weights on average (and unit weight was the - # old buggy behavior). - avgPixelSigma = 0.004**0.5 - config.measurement.plugins["modelfit_CModel"].initial.weightsMultiplier = avgPixelSigma - config.measurement.plugins["modelfit_CModel"].exp.weightsMultiplier = avgPixelSigma - config.measurement.plugins["modelfit_CModel"].dev.weightsMultiplier = avgPixelSigma - except (KeyError, ImportError): print("Cannot import lsst.meas.modelfit: disabling CModel measurements")
            Hide
            jbosch Jim Bosch added a comment - - edited

            I've just uploaded some plots showing CModel (r-i) color vs. Afterburner (r-i) color and CModel (i) vs. Kron (i). These are all limited to isolated objects.

            The bottom line is:

            • Kron vs. CModel has very weird features that will be fixed by accepting the CModel bug fix.
            • Scatter in color (compared to afterburner) increases slightly with the bug fixed, but it's not obviously more than we'd expect just from the fact that CModel is supposed to get more different from afterburner with this change.
            Show
            jbosch Jim Bosch added a comment - - edited I've just uploaded some plots showing CModel (r-i) color vs. Afterburner (r-i) color and CModel (i) vs. Kron (i). These are all limited to isolated objects. The bottom line is: Kron vs. CModel has very weird features that will be fixed by accepting the CModel bug fix. Scatter in color (compared to afterburner) increases slightly with the bug fixed, but it's not obviously more than we'd expect just from the fact that CModel is supposed to get more different from afterburner with this change.
            Hide
            jbosch Jim Bosch added a comment -

            Changes look good, and I think the switch is as vetted as it's gonna get in the near future. I'll email hsc_software to that effect shortly.

            Show
            jbosch Jim Bosch added a comment - Changes look good, and I think the switch is as vetted as it's gonna get in the near future. I'll email hsc_software to that effect shortly.
            Hide
            price Paul Price added a comment -

            Thanks Jim.

            Merged to master.

            Show
            price Paul Price added a comment - Thanks Jim. Merged to master.

              People

              • Assignee:
                price Paul Price
                Reporter:
                price Paul Price
                Reviewers:
                Jim Bosch
                Watchers:
                Hsin-Fang Chiang, Jim Bosch, Paul Price
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: