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

Add unnormalized (but continuous) version of PixelScaleBoundedField

    XMLWordPrintable

    Details

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

      Description

      PixelScaleBoundedField is a bit inconvenient to use in full-focal-plane contexts (the only contexts in which we expect to use it) because it normalizes by the area of the origin pixel in order to yield a unitless quantity, and that origin pixel is a per-CCD quantity, resulting in a discontinuous combined field over the focal plane.

      Duplicate and adjust it to define a new PixelAreaBoundedField class; I'll RFC deprecating and removing the original in a separate RFC to avoid worrying about code duplication in the interim.  Also make sure the new class is persistable (the original is not).

       

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            Eli Rykoff, the new PixelAreaBoundedField is now ready to go on this ticket branch if you'd like to play with it (the branch also includes ProductBoundedField).  I'll defer deprecating PixelScaleBoundedField until after the RFC, since that blocks this from merging anyway.

            I also took a look at the evaluation code, and as long as you're getting a full image without using the optional interpolation arguments or calling the flattened numpy interface directly, it should be about as fast as we can make it without really working on optimizing it.  Hopefully that will be fast enough.  The interpolation code path is probably horrifically slow right now (as is PixelScaleBoundedField in all contexts, I'd guess), and I've documented that on DM-22071 (which I'm not considering a high priority).

            Show
            jbosch Jim Bosch added a comment - Eli Rykoff , the new PixelAreaBoundedField is now ready to go on this ticket branch if you'd like to play with it (the branch also includes ProductBoundedField ).  I'll defer deprecating PixelScaleBoundedField until after the RFC, since that blocks this from merging anyway. I also took a look at the evaluation code, and as long as you're getting a full image without using the optional interpolation arguments or calling the flattened numpy interface directly, it should be about as fast as we can make it without really working on optimizing it.  Hopefully that will be fast enough.  The interpolation code path is probably horrifically slow right now (as is PixelScaleBoundedField in all contexts, I'd guess), and I've documented that on DM-22071 (which I'm not considering a high priority).
            Hide
            erykoff Eli Rykoff added a comment -

            I have taken a look, and it seems to do exactly what I need it to do, which is great! But ...

            I know you were worried about the speed of it, and you were right. On my workstation (which is many years old and not the fastest on the block, but it does get the job done) I've done a simple benchmarking of doing 8000000 points for a 2nd degree ChebyshevBoundedField and the new PixelAreaBoundedField. (Representing a 2k x 4k hsc ccd).
            And evaluating the ChebyshevBoundedField takes ~0.37 seconds (and I haven't checked but I assume the product of two such fields takes twice that).
            But evaluating the PixelAreaBoundedField takes ~35 seconds. So this would slow down coaddition significantly unless we can speed it up (vectorize as suggested in DM-22071 ?).

            Show
            erykoff Eli Rykoff added a comment - I have taken a look, and it seems to do exactly what I need it to do, which is great! But ... I know you were worried about the speed of it, and you were right. On my workstation (which is many years old and not the fastest on the block, but it does get the job done) I've done a simple benchmarking of doing 8000000 points for a 2nd degree ChebyshevBoundedField and the new PixelAreaBoundedField . (Representing a 2k x 4k hsc ccd). And evaluating the ChebyshevBoundedField takes ~0.37 seconds (and I haven't checked but I assume the product of two such fields takes twice that). But evaluating the PixelAreaBoundedField takes ~35 seconds. So this would slow down coaddition significantly unless we can speed it up (vectorize as suggested in DM-22071 ?).
            Hide
            jbosch Jim Bosch added a comment -

            I've done the deprecation using the existing lsst.utils.deprecated_pybind11 function, which wasn't documented to work on classes but basically does (the warning message isn't ideal, as it sounds as if the constructor rather than the class is what's deprecated, but it's better than nothing).  I've opened DM-22207 to improve the documentation, but that needn't block this ticket.

            Show
            jbosch Jim Bosch added a comment - I've done the deprecation using the existing  lsst.utils.deprecated_pybind11 function, which wasn't documented to work on classes but basically does (the warning message isn't ideal, as it sounds as if the constructor rather than the class is what's deprecated, but it's better than nothing).  I've opened DM-22207 to improve the documentation, but that needn't block this ticket.
            Hide
            jbosch Jim Bosch added a comment -

            Eli Rykoff, up for taking this review, too?  Code is ready for review, and Jenkins is running.  PR is https://github.com/lsst/afw/pull/499.

            Show
            jbosch Jim Bosch added a comment - Eli Rykoff , up for taking this review, too?  Code is ready for review, and Jenkins is running.  PR is https://github.com/lsst/afw/pull/499 .
            Hide
            erykoff Eli Rykoff added a comment -

            See minor comments on PR.

            Show
            erykoff Eli Rykoff added a comment - See minor comments on PR.

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Eli Rykoff
              Watchers:
              Eli Rykoff, Jim Bosch
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: