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

Weighting in photometric calibration is incorrect

    XMLWordPrintable

Details

    • Bug
    • Status: Done
    • Resolution: Done
    • None
    • meas_photocal
    • None

    Description

      Dominique points out that the zero point calibration uses errors not inverse errors to calculate the zero point. git annotate reveals:

      24c9149f python/lsst/meas/photocal/PhotoCal.py (Robert Lupton the Good 2010-12-13 05:03:12 +0000 353) return np.average(dmag, weights=dmagErr), np.std(dmag, ddof=1), len(dmag)

      Please fix this. At the same time, we should add a config parameter to soften the errors.

      Attachments

        Issue Links

          Activity

            boutigny Dominique Boutigny added a comment - - edited

            I think that the error is also incorrect (np.std(dmag)) I think that it should be something like :
            zp, sum = np.average(dmag, weights=1/dmagErr**2,returned=True)
            sigma = np.sqrt(1.0/sum)

            Assuming that the dmagErr values are correct

            boutigny Dominique Boutigny added a comment - - edited I think that the error is also incorrect (np.std(dmag)) I think that it should be something like : zp, sum = np.average(dmag, weights=1/dmagErr**2,returned=True) sigma = np.sqrt(1.0/sum) Assuming that the dmagErr values are correct
            jbosch Jim Bosch added a comment -

            I'm just adopting boutigny's proposed solution, as it seems correct, or at least vastly more correct than what we have now. I do wonder a bit whether we should be doing this average in flux units rather than magnitude units, but I think that's beyond the scope of this issue.

            rhl, by "soften the errors", do you just mean "add a constant in quadrature" (with the constant coming from the config)?

            jbosch Jim Bosch added a comment - I'm just adopting boutigny 's proposed solution, as it seems correct, or at least vastly more correct than what we have now. I do wonder a bit whether we should be doing this average in flux units rather than magnitude units, but I think that's beyond the scope of this issue. rhl , by "soften the errors", do you just mean "add a constant in quadrature" (with the constant coming from the config)?

            Probably not exactly in the scope of this ticket, but the main problem is that the Photocal fit is not taking into account the error on the flux of the reference stars. In some region of the SDSS catalog (CFHTLS D3 for instance) they can be quite large.

            boutigny Dominique Boutigny added a comment - Probably not exactly in the scope of this ticket, but the main problem is that the Photocal fit is not taking into account the error on the flux of the reference stars. In some region of the SDSS catalog (CFHTLS D3 for instance) they can be quite large.
            jbosch Jim Bosch added a comment -

            This issue will address some (but not all) of the problems discussed in DM-2308.

            jbosch Jim Bosch added a comment - This issue will address some (but not all) of the problems discussed in DM-2308 .
            jbosch Jim Bosch added a comment -

            Simon, do you have time for a small review? This fixes a big but simple problem in the Photocal algorithm, along with a number of minor fixes and cleanups. All changes on tickets/DM-2423 of pipe_tasks.

            jbosch Jim Bosch added a comment - Simon, do you have time for a small review? This fixes a big but simple problem in the Photocal algorithm, along with a number of minor fixes and cleanups. All changes on tickets/ DM-2423 of pipe_tasks.
            jbosch Jim Bosch added a comment -

            Simon is super busy. rowen, could you take on this review? It's a pretty small one.

            jbosch Jim Bosch added a comment - Simon is super busy. rowen , could you take on this review? It's a pretty small one.

            photoCal.py

            The changes look helpful.

            One small request: please change the final return to specify each value on its own line, as per the original code. I feel that makes it easier for a user to see exactly what named values are in the returned struct.

            testPhotocal.py

            The changes look helpful.

            rowen Russell Owen added a comment - photoCal.py The changes look helpful. One small request: please change the final return to specify each value on its own line, as per the original code. I feel that makes it easier for a user to see exactly what named values are in the returned struct. testPhotocal.py The changes look helpful.
            jbosch Jim Bosch added a comment -

            One small request: please change the final return to specify each value on its own line, as per the original code. I feel that makes it easier for a user to see exactly what named values are in the returned struct.

            I find that a bit unnecessarily verbose, but I also don't care much. Will do.

            jbosch Jim Bosch added a comment - One small request: please change the final return to specify each value on its own line, as per the original code. I feel that makes it easier for a user to see exactly what named values are in the returned struct. I find that a bit unnecessarily verbose, but I also don't care much. Will do.
            jbosch Jim Bosch added a comment -

            Merged to master.

            jbosch Jim Bosch added a comment - Merged to master.

            Yes.

            rhl Robert Lupton added a comment - Yes.

            People

              jbosch Jim Bosch
              rhl Robert Lupton
              Russell Owen
              Dominique Boutigny, Jim Bosch, Rachel Mandelbaum, Robert Lupton, Russell Owen, Simon Krughoff (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.