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

LengthError in making psfCandidate

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Story Points:
      4
    • Sprint:
      AP S19-4, AP S19-5
    • Team:
      Alert Production

      Description

      I came across the following log record from processCcd-ing HSC data --id visit=38950 ccd=64

      processCcd.charImage.measurePsf.makePsfCandidates WARN: Failed to make a psfCandidate from star 33458070113746946:
        File "src/image/Image.cc", line 84, in static lsst::afw::image::ImageBase<PixelT>::_view_t lsst::afw::image::ImageBase<PixelT>::_makeSubView(const Extent2I&, const Extent2I&, const _view_t&) [with PixelT = float; lsst::afw::image::ImageBase<PixelT>::_view_t = boost::gil::image_view<boost::gil::memory_based_2d_locator<boost::gil::memory_based_step_iterator<boost::gil::pixel<float, boost::gil::layout<boost::mpl::vector1<boost::gil::gray_color_t> > >*> > >; lsst::geom::Extent2I = lsst::geom::Extent<int, 2>]
          Box2I(Point2I(470,-1),lsst::geom::Extent2I(21,21)) doesn't fit in image 2048x4176 {0}
        File "src/PsfCandidate.cc", line 182, in std::shared_ptr<lsst::afw::image::MaskedImage<ImagePixelT1, int, float> > lsst::meas::algorithms::PsfCandidate<PixelT>::extractImage(unsigned int, unsigned int) const [with PixelT = float]
          Extracting image of PSF candidate {1}
      lsst::pex::exceptions::LengthError: 'Box2I(Point2I(470,-1),lsst::geom::Extent2I(21,21)) doesn't fit in image 2048x4176 {0}; Extracting image of PSF candidate {1}'
      

      Then the processing carries on and finished processCcd just fine. This "LengthError" didn't appear by using w_2018_17 but appeared using w_2018_18. It still appears with the most recent weekly of w_2018_28.

        Attachments

          Issue Links

            Activity

            Hide
            Parejkoj John Parejko added a comment - - edited

            Thank you, Hsin-Fang Chiang, that was helpful.

            I've tagged three other tickets that are directly related to this: the fundamental problem is that PsfCandidate has static members, so candidates near the border are forced to have the same size as all the others, and thus sometimes get rejected because the box goes off the edge. The tickets I marked "related" discuss various aspects of this point, and why it might be non-trivial to fix. We don't have a ticket/epic to redo PsfCandidate from the ground up, but we probably should: there's a lot of very old code in it, and some questionable design choices.

            The reason this warning started appearing after DM-14102 is that Chris Morrison [X] and I changed the log.debug statement in the except block to log.warn: if an exception is being caught, we thought it should probably be logged in a more visible manner, given that it was a blanket "catch everything". One approach to handling this exception in a less visible manner would be to have makePsfCandidate either throw a custom exception when the candidate falls off of the image, or treat the LengthError that is currently raised at PsfCandidate.cc::182 as said custom exception, and log that message at a lower level. I'm not extremely keen on the latter, given that there are a variety of ways that other can throw LengthError, but making custom C++ exceptions that python understands for the former isn't as simple as I'd hope. Suggested approaches welcome.

            I'll leave it to John Swinbank to decide which of the above "relates" should be "duplicates", etc.

            Show
            Parejkoj John Parejko added a comment - - edited Thank you, Hsin-Fang Chiang , that was helpful. I've tagged three other tickets that are directly related to this: the fundamental problem is that PsfCandidate has static members, so candidates near the border are forced to have the same size as all the others, and thus sometimes get rejected because the box goes off the edge. The tickets I marked "related" discuss various aspects of this point, and why it might be non-trivial to fix. We don't have a ticket/epic to redo PsfCandidate from the ground up, but we probably should: there's a lot of very old code in it, and some questionable design choices. The reason this warning started appearing after DM-14102 is that Chris Morrison [X] and I changed the log.debug statement in the except block to log.warn : if an exception is being caught, we thought it should probably be logged in a more visible manner, given that it was a blanket "catch everything". One approach to handling this exception in a less visible manner would be to have makePsfCandidate either throw a custom exception when the candidate falls off of the image, or treat the LengthError that is currently raised at PsfCandidate.cc::182 as said custom exception, and log that message at a lower level. I'm not extremely keen on the latter, given that there are a variety of ways that other can throw LengthError , but making custom C++ exceptions that python understands for the former isn't as simple as I'd hope. Suggested approaches welcome. I'll leave it to John Swinbank to decide which of the above "relates" should be "duplicates", etc.
            Hide
            swinbank John Swinbank added a comment - - edited

            I suggest that:

            • John Parejko has explained the source of the warning that Hsin-Fang Chiang is seeing, and that it does not represent a regression (except in the amount of text on screen) over previous versions. That is sufficient to close this ticket as done).
            • All of John Parejko, Paul Price and Perry Gee (per comments on DM-7040) agree that this code needs substantial reworking, which should include making this warning vanish. I will file a ticket to capture that work.
            • Since (as far as we know) the fragility of the current code isn't directly causing problems with scientific results, it's hard to prioritise the work relative to other technical debt. Rather than try to decide on that here, I suggest that Eric Bellm, Jim Bosch, Yusra AlSayyad and I discuss this as part of regular sprint planning.

            Hsin-Fang Chiang, are you happy with that approach? If neither you nor anybody else objects in the next few hours, I think John Parejko can mark this ticket as done.

            Show
            swinbank John Swinbank added a comment - - edited I suggest that: John Parejko has explained the source of the warning that Hsin-Fang Chiang is seeing, and that it does not represent a regression (except in the amount of text on screen) over previous versions. That is sufficient to close this ticket as done). All of John Parejko , Paul Price and Perry Gee (per comments on DM-7040 ) agree that this code needs substantial reworking, which should include making this warning vanish. I will file a ticket to capture that work. Since (as far as we know) the fragility of the current code isn't directly causing problems with scientific results, it's hard to prioritise the work relative to other technical debt. Rather than try to decide on that here, I suggest that Eric Bellm , Jim Bosch , Yusra AlSayyad and I discuss this as part of regular sprint planning. Hsin-Fang Chiang , are you happy with that approach? If neither you nor anybody else objects in the next few hours, I think John Parejko can mark this ticket as done.
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

             from me

            Show
            hchiang2 Hsin-Fang Chiang added a comment -  from me
            Hide
            Parejkoj John Parejko added a comment -

            See the comments from John Swinbank and me above: this is just a previously-squashed message now being more visible. Fixes to the underlying issues with PsfCandidate would be a whole epic itself (that John Swinbank should probably file?).

            Show
            Parejkoj John Parejko added a comment - See the comments from John Swinbank and me above: this is just a previously-squashed message now being more visible. Fixes to the underlying issues with PsfCandidate would be a whole epic itself (that John Swinbank should probably file?).
            Hide
            swinbank John Swinbank added a comment -

            Filed DM-19155 to capture the work of designing a replacement. I don't think we want this to be a whole epic, at least until a design has been proposed — at that point, maybe implementation becomes an epic.

            Changing resolution of this ticket to “Done”, since effort was expended and a resolution was reached (even if it didn't result in code changes).

            Show
            swinbank John Swinbank added a comment - Filed DM-19155 to capture the work of designing a replacement. I don't think we want this to be a whole epic, at least until a design has been proposed — at that point, maybe implementation becomes an epic. Changing resolution of this ticket to “Done”, since effort was expended and a resolution was reached (even if it didn't result in code changes).

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              hchiang2 Hsin-Fang Chiang
              Watchers:
              Hsin-Fang Chiang, Jim Bosch, John Parejko, John Swinbank, Paul Price
              Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.