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

Backwards compatibility API for Calib->PhotoCalib transition

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      The API of the new PhotoCalib object is entirely different from the old Calib API: it is defined in terms of a flux calibration factor instead of a magnitude zero point, methods are named e.g. instFluxToMagnitude instead of getMagnitude, and it never raises an exception when given a negative instrumental flux. To ease the transition to PhotoCalib, I propose that PhotoCalib gain the following deprecated methods from Calib until v18.0:

          /**
           * Return an instrumental flux and error (in ADU) given magnitude and magnitude error.
           *
           * Assumes that the errors are small and uncorrelated.
           *
           * @param mag The AB magnitude of the object.
           * @param magErr The error in the magnitude.
           */
          std::pair<double, double> getFlux(double const mag, double const magErr) const;
          ndarray::Array<double, 1> getFlux(ndarray::Array<double const, 1> const& mag) const;
          std::pair<ndarray::Array<double, 1>, ndarray::Array<double, 1>> getFlux(
                  ndarray::Array<double const, 1> const& mag, ndarray::Array<double const, 1> const& magErr) const;
       
          /**
           * Return a magnitude and magnitude error given instrumental flux and error (in ADU).
           *
           * @param instFlux The measured instFlux of the object (ADU).
           * @param instFluxErr The error in the measured instFlux (ADU).
           */
          std::pair<double, double> getMagnitude(double const instFlux, double const instFluxErr) const;
          ndarray::Array<double, 1> getMagnitude(ndarray::Array<double const, 1> const& instFlux) const;
          std::pair<ndarray::Array<double, 1>, ndarray::Array<double, 1>> getMagnitude(
                  ndarray::Array<double const, 1> const& instFlux,
                  ndarray::Array<double const, 1> const& instFluxErr) const;
      

      Other methods on Calib that we will not provide backwards-compatible deprecated methods for:

      • setFluxMag0: Invalid; PhotoCalib is immutable.
      • getFluxMag0: PhotoCalib has getInstFluxAtZeroMagnitude, which only returns the 0-point and not the 0-point error, which has a non-trivial relationship with the calibration error. This was most commonly used to construct a new Calib, about which see the next point.
      • Construction from a "instFlux at zero magnitude" double: this cannot be distinguished from the existing calibrationMean constructor. If such functionality is strongly desired, we could add a makePhotoCalibFromZeroPoint (or similar) free function, but I doubt it is necessary: users rarely have to construct their own photoCalib, and PhotoCalib will be able to read older Exposure files that contain persisted Calibs.
      • get/setThrowOnNegativeFlux: Invalid; PhotoCalib is immutable and never throws in this manner.
      • operator*= and operator/=: Invalid; PhotoCalib is immutable. We could provide operator* and operator/ though (returning a new PhotoCalib), if this functionality was important.

        Attachments

          Issue Links

            Activity

            Hide
            Parejkoj John Parejko added a comment -

            I am not yet clear about how/if we'll actually remove deprecated methods. I assume that a user will be able to disable the deprecation messages.

            I would propose that we remove of all the Calib-related APIs (including the above additions to PhotoCalib, and the getCalib() methods in Exposure and ExposureInfo) either just before releasing v18, or immediately after that.

            Show
            Parejkoj John Parejko added a comment - I am not yet clear about how/if we'll actually remove deprecated methods. I assume that a user will be able to disable the deprecation messages. I would propose that we remove of all the Calib -related APIs (including the above additions to PhotoCalib , and the getCalib() methods in Exposure and ExposureInfo ) either just before releasing v18, or immediately after that.
            Hide
            rhl Robert Lupton added a comment -

            I think that this is fine (with Jim's proposed addition). I don't understand why getFluxMag0 cannot return a the error as well as the zero point, but if it is only used to create new {{Calib}}s (which seems likely) maybe it's not worth reporting.

            I am not yet clear about how/if we'll actually remove deprecated methods. I assume that a user will be able to disable the deprecation messages.

            Show
            rhl Robert Lupton added a comment - I think that this is fine (with Jim's proposed addition). I don't understand why getFluxMag0 cannot return a the error as well as the zero point, but if it is only used to create new {{Calib}}s (which seems likely) maybe it's not worth reporting. I am not yet clear about how/if we'll actually remove deprecated methods. I assume that a user will be able to disable the deprecation messages.
            Hide
            jbosch Jim Bosch added a comment - - edited

            I think this is broadly fine.  One possible adjustment I think I'd recommend (apologies for not coming up with this when I was asked to comment on an earlier version of this proposal): we could stil retain (and of course deprecate) get/setThrowOnNegativeFlux as no-ops; I doubt these are used often (if at all) in pipeline code, but they could be quite common in notebooks and other analysis code.

            Show
            jbosch Jim Bosch added a comment - - edited I think this is broadly fine.  One possible adjustment I think I'd recommend (apologies for not coming up with this when I was asked to comment on an earlier version of this proposal): we could stil retain (and of course deprecate) get/setThrowOnNegativeFlux as no-ops; I doubt these are used often (if at all) in pipeline code, but they could be quite common in notebooks and other analysis code.
            Hide
            swinbank John Swinbank added a comment - - edited

            I'd like the DM-CCB to approve this suggestion. However, I suggest that the DM-CCB should invite expert opinion from Robert Lupton and/or Jim Bosch before making a ruling.

            Show
            swinbank John Swinbank added a comment - - edited I'd like the DM-CCB to approve this suggestion. However, I suggest that the DM-CCB should invite expert opinion from Robert Lupton and/or Jim Bosch before making a ruling.

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              Parejkoj John Parejko
              Watchers:
              Jim Bosch, John Parejko, John Swinbank, Leanne Guy, Paul Price, Robert Lupton, Tim Jenness, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  CI Builds

                  No builds found.