Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-437

Drop GaussianCentroid

    XMLWordPrintable

Details

    • RFC
    • Status: Implemented
    • Resolution: Done
    • DM
    • None

    Description

      GaussianCentroid is a measurement algorithm in meas_base that fits a 2D Gaussian to sources to determine the centroid. We have recently discovered that the code has bugs (DM-13299).

      I propose dropping GaussianCentroid, which will save on maintenance effort. Since SdssCentroid performs at least as well, is faster, and (as far as I am aware) is always preferred by developers and users, I believe the only reason not to drop GaussianCentroid is to keep a second centroiding algorithm in the main stack. I suggest that if we find that we need a second centroiding algorithm in the future, the code could be resurrected from git.

      Attachments

        Issue Links

          Activity

            price Paul Price added a comment -

            A search of my e-mail revealed the attached plot you sent 2012-10-08, based on DECam 30-sec images processed with the LSST stack and SExtractor:

            And the captions:

            Top Panel: The black lines (filled symbol for X, open for Y) come from
            the SExtractor solution and the magenta from centroid_naive.
            Not a surprise. The FITS binary table header comments say
            that the "naive" algorithm is "unweighted 3x3 first moment".
            It is well known that functional fits produce better centroids
            than image moments. This panel just quantifies this.

            Middle Panel: X solutions (i.e., the 2K direction of the DECam CCDs)
            for the four remaining algorithms. Note that the scale
            is expanded compared to the top panel. Black is for
            SExtractor, red for SDSS, green for GAUSS, and blue for
            SHAPE.

            Bottom Panel: Y (i.e., the 4K direction) solutions with same scale and
            symbols as middle panel.

            price Paul Price added a comment - A search of my e-mail revealed the attached plot you sent 2012-10-08, based on DECam 30-sec images processed with the LSST stack and SExtractor: And the captions: Top Panel: The black lines (filled symbol for X, open for Y) come from the SExtractor solution and the magenta from centroid_naive. Not a surprise. The FITS binary table header comments say that the "naive" algorithm is "unweighted 3x3 first moment". It is well known that functional fits produce better centroids than image moments. This panel just quantifies this. Middle Panel: X solutions (i.e., the 2K direction of the DECam CCDs) for the four remaining algorithms. Note that the scale is expanded compared to the top panel. Black is for SExtractor, red for SDSS, green for GAUSS, and blue for SHAPE. Bottom Panel: Y (i.e., the 4K direction) solutions with same scale and symbols as middle panel.
            dmonet David Monet added a comment -

            Gasp!  I am always humiliated when I see how clever I was in the past and how far I have fallen.  Yes, it appears that a reasonable test was done courtesy of Paul, Robert, and others.  The e-mail thread also discusses the astrometric impact of short exposures taken with DECam that found their way into the LSST SRD.  Perhaps I should take the Action Item to prepare a better document than the 10 year old e-mail thread and dump it somewhere in the LSST documentation quagmire.  The Executive Summary seems to be that Paul can proceed to dump the columns for a second centroid if he promises that the code has not changed in the last decade.

            dmonet David Monet added a comment - Gasp!  I am always humiliated when I see how clever I was in the past and how far I have fallen.  Yes, it appears that a reasonable test was done courtesy of Paul, Robert, and others.  The e-mail thread also discusses the astrometric impact of short exposures taken with DECam that found their way into the LSST SRD.  Perhaps I should take the Action Item to prepare a better document than the 10 year old e-mail thread and dump it somewhere in the LSST documentation quagmire.  The Executive Summary seems to be that Paul can proceed to dump the columns for a second centroid if he promises that the code has not changed in the last decade.
            price Paul Price added a comment -

            I had a quick look through the git commit logs for GaussianCentroid.cc (in meas_base and meas_algorithms) and I don't see any functional changes since 2012.

            price Paul Price added a comment - I had a quick look through the git commit logs for GaussianCentroid.cc (in meas_base and meas_algorithms ) and I don't see any functional changes since 2012.
            price Paul Price added a comment -

            Adopted since dmonet agrees and no-one dissents.

            price Paul Price added a comment - Adopted since dmonet agrees and no-one dissents.
            tjenness Tim Jenness added a comment -

            price I think this ticket can be marked as implemented.

            tjenness Tim Jenness added a comment - price I think this ticket can be marked as implemented.

            People

              price Paul Price
              price Paul Price
              David Monet, John Parejko, John Swinbank, Paul Price, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                Jenkins

                  No builds found.