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

Default Psf position argument considered harmful

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Adopted
    • Resolution: Unresolved
    • Component/s: DM
    • Labels:
      None

      Description

      Most of the Psf methods that can compute something at a particular point in an image allow for the actual position argument to be omitted, in which case an "average" position is used by default. See many of the methods in Psf.h  This pattern can easily lead to problems when either:

      In cases where the position really is intended to be the average position, it's easy enough to query the Psf object that the programmer already has access to, e.g.:

      psf.computeImage(psf.getAveragePosition())

      Since this pattern appears to be dangerous and unnecessary, I propose to deprecate the no-position-argument interface in each of:

      Psf::computeImage
      Psf::computeKernelImage
      Psf::computePeak
      Psf::computeApertureFlux
      Psf::computeShape
      Psf::getLocalKernel
      Psf::computeBBox
      Psf::computeImageBBox
      Psf::computeKernelBBox

      and their python equivalents.

      (I haven't tried, but I'm guessing we could implement this in c++ by swapping the default parametered methods for overloads).

      Second, Psf::getAveragePosition() has a virtual default implementation, which just returns Point2D(). It is up to derived classes to override this if required, which should be the case for all non-constant Psf models.  However, I'm pretty sure I didn't do this right in meas_extensions_piff.  So part (b) of this RFC is to add a Psf::_averagePosition member variable and require constructors to initialize it. This is effectively already done in a number of derived classes, e.g.,

      Making it mandatory will prevent any future Psf derived classes from naively inheriting a default implementation.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              jmeyers314 Joshua Meyers
              Reporter:
              jmeyers314 Joshua Meyers
              Watchers:
              Arun Kannawadi, Eli Rykoff, Joshua Meyers, Krzysztof Findeisen, Tim Jenness
              Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Planned End:

                  Jenkins

                  No builds found.