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

Add Classes of MeasurementError

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_base
    • Labels:

      Description

      Some measurement failures are global for a whole exposure, such as a missing Psf or Wcs. The framework currently does not distinguish this from a failure in a single measurement.

      Should add a new subclass of MeasurementError which can be thrown in these cases.

      Should also add configuration option to the measurement framework to determine what should be done with this type of error.

      We should also add an exception class and associated how-to-handle config for problems that indicate that something has gone wrong in pre-measurement processing, such as NaNs in the image.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            The kinds of errors to be handled in this ticket actually fall into two categories, which we should deal with differently:

            • For problems that cause all measurements of a certain type to fail on an exposure, because something is wrong with that exposure that can be determined up front (e.g. no PSF), we should add a way (perhaps a plugin class member) a plugin can use to declare which properties of an exposure must be met for it to run. The measurement task could then check those properties when processing a new exposure, and then raise an exception before calling any plugins. (I do think we want to raise a fatal exception here, because it indicates either a logic bug in the configuration or a pretty serious failure in a previous processing stage. If we need to, we can have a config option to proceed anyway, and then have the measurement task skip the plugins that it knows can't handle that exposure.
            • For common problems that can only be determined when an algorithm is running (including looking for e.g. NaNs, since we don't want to do that up front over the entire exposure), we should define a custom exception class for that kind of failure that can be thrown by the plugins. This should inherit from MeasurementError and hence always set a bit. In some cases we may want to have config options on the task to control whether to suppress, warn, or propagate up these exceptions.
            Show
            jbosch Jim Bosch added a comment - The kinds of errors to be handled in this ticket actually fall into two categories, which we should deal with differently: For problems that cause all measurements of a certain type to fail on an exposure, because something is wrong with that exposure that can be determined up front (e.g. no PSF), we should add a way (perhaps a plugin class member) a plugin can use to declare which properties of an exposure must be met for it to run. The measurement task could then check those properties when processing a new exposure, and then raise an exception before calling any plugins. (I do think we want to raise a fatal exception here, because it indicates either a logic bug in the configuration or a pretty serious failure in a previous processing stage. If we need to, we can have a config option to proceed anyway, and then have the measurement task skip the plugins that it knows can't handle that exposure. For common problems that can only be determined when an algorithm is running (including looking for e.g. NaNs, since we don't want to do that up front over the entire exposure), we should define a custom exception class for that kind of failure that can be thrown by the plugins. This should inherit from MeasurementError and hence always set a bit. In some cases we may want to have config options on the task to control whether to suppress, warn, or propagate up these exceptions.
            Hide
            pgee Perry Gee added a comment -

            This is actually a pretty easy review. I'm giving it to you so that you can see what the behavior is going to be and can comment on the exception handling.

            checkin was to u/pgee/DM-461

            pgeepc2:/sandbox2/pgee/mylsst7/Linux64/mb12> git diff --stat origin/master
            include/lsst/meas/base/GaussianFlux.h | 2 –
            include/lsst/meas/base/Inputs.h | 15 +++++++++++++--
            include/lsst/meas/base/PsfFlux.h | 2 –
            include/lsst/meas/base/SdssCentroid.h | 2 –
            include/lsst/meas/base/exceptions.h | 26 ++++++++++++++++++++++++++
            python/lsst/meas/base/base.py | 2 +-
            python/lsst/meas/base/baseLib.i | 1 +
            src/GaussianFlux.cc | 5 ++---
            src/PsfFlux.cc | 5 ++---
            src/SdssCentroid.cc | 5 ++---
            tests/testGaussianFluxBasic.py | 1 -
            tests/testPsfFlux.py | 5 +----
            tests/testPsfFluxBasic.py | 1 -
            tests/testSdssCentroidBasic.py | 1 -
            14 files changed, 48 insertions, 25 deletions

            Show
            pgee Perry Gee added a comment - This is actually a pretty easy review. I'm giving it to you so that you can see what the behavior is going to be and can comment on the exception handling. checkin was to u/pgee/ DM-461 pgeepc2:/sandbox2/pgee/mylsst7/Linux64/mb12> git diff --stat origin/master include/lsst/meas/base/GaussianFlux.h | 2 – include/lsst/meas/base/Inputs.h | 15 +++++++++++++-- include/lsst/meas/base/PsfFlux.h | 2 – include/lsst/meas/base/SdssCentroid.h | 2 – include/lsst/meas/base/exceptions.h | 26 ++++++++++++++++++++++++++ python/lsst/meas/base/base.py | 2 +- python/lsst/meas/base/baseLib.i | 1 + src/GaussianFlux.cc | 5 ++--- src/PsfFlux.cc | 5 ++--- src/SdssCentroid.cc | 5 ++--- tests/testGaussianFluxBasic.py | 1 - tests/testPsfFlux.py | 5 +---- tests/testPsfFluxBasic.py | 1 - tests/testSdssCentroidBasic.py | 1 - 14 files changed, 48 insertions , 25 deletions
            Hide
            jbosch Jim Bosch added a comment -

            Looks good to merge, no comments.

            Show
            jbosch Jim Bosch added a comment - Looks good to merge, no comments.
            Hide
            jbosch Jim Bosch added a comment -

            Oops, for some reason I thought this review was assigned to me.

            Show
            jbosch Jim Bosch added a comment - Oops, for some reason I thought this review was assigned to me.
            Hide
            rowen Russell Owen added a comment -

            I agree with Jim. This looks great. Please merge.

            Show
            rowen Russell Owen added a comment - I agree with Jim. This looks great. Please merge.

              People

              • Assignee:
                pgee Perry Gee
                Reporter:
                pgee Perry Gee
                Reviewers:
                Russell Owen
                Watchers:
                Jim Bosch, Perry Gee, Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel