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

Add sphgeom.Pixelization for HEALPix

    XMLWordPrintable

    Details

    • Type: Story
    • Status: In Progress
    • Resolution: Unresolved
    • Fix Version/s: None
    • Component/s: sphgeom
    • Story Points:
      10
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      Our sphgeom provides a nice generic interface for spherical pixelizations, with implementations for HTM, Q3C, and a custom modification of Q3C.  By delegating the work to the public HEALPix C++ library, we could easily add HEALPix to the mix, and write downstream mask manipulation code without assuming a particular pixelization.  It'd also give us an easy way to compare the performance of different pixelizations.

      The HEALPix C++ library is packaged by conda-forge as healpix_cxx, and at least right now it installs without any trouble into our environment. When this ticket is nearing completion we can RFC getting that into our standard installs. We should then either make HEALPix an optional dependency of sphgeom or (probably better) put a new subclass of sphgeom::Pixelization in a new package that depends on both HEALPix and sphgeom; we want to make sure the current sphgeom functionality continues to be available without additional dependencies.

      It looks like the tricky part of implementing that new Pixelization will be determining whether the pixel method can just return a sphgeom::ConvexPolygon with the four vertices of the pixel (i.e. under what conditions do the non-geodesic HEALPix boundaries extend beyond the geodesic boundaries of a ConvexPolygon with the same vertices instead lying completely within them?).  I've only taken a quick glance, but it looks like the HEALPix library includes routines for directly implementing all of Pixelization's other virtual methods.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            I'm starting this now, in my 20% time, because it looks fun and I'd like a bit of a break from butler/python (in my 20% time, or whatever fraction of that I can actually carve out these days).

            Here's my plan:

            1. Add stubs for HEALPixels as a sphgeom::Region subclass to sphgeom itself. This will be an abstract class with no implementations in sphgeom, but we need it defined there so it can participate in Region's visitor-pattern double-dispatch system for topological relationships.
            2. Write some Rust bindings for sphgeom.
            3. Write a HEALPix sphpgeom::Pixelization implementation in Rust, delegating the real work to the CDS HEALPix Rust crate.
            4. Put all of these (including sphgeom itself) on conda-forge.
            5. Start using sphgeom via conda-forge in other DM code.

            There is of course a HEALPix C++ library as well, and we could replace steps 2-3 with one that uses that implements the Region and Pixelization interfaces in a downstream C++ package. This would get this up and running faster, but I have heard rumblings that there are problems with its region intersections that the CDS Rust implementation does not have. More importantly, steps 2-3 are what makes this look fun to me.

            If this effort doesn't converge before having HEALPix in sphgeom becomes a higher priority for LSST, we can go back to trying to do this completely in C++. All the work in Step 1 carries over directly, and this design should even let someone have multiple HEALPix sphgeom implementations installed at the same time.

            Show
            jbosch Jim Bosch added a comment - I'm starting this now, in my 20% time, because it looks fun and I'd like a bit of a break from butler/python (in my 20% time, or whatever fraction of that I can actually carve out these days). Here's my plan: Add stubs for HEALPixels as a sphgeom::Region subclass to sphgeom itself. This will be an abstract class with no implementations in sphgeom, but we need it defined there so it can participate in Region's visitor-pattern double-dispatch system for topological relationships. Write some Rust bindings for sphgeom. Write a HEALPix sphpgeom::Pixelization implementation in Rust, delegating the real work to the CDS HEALPix Rust crate. Put all of these (including sphgeom itself) on conda-forge. Start using sphgeom via conda-forge in other DM code. There is of course a HEALPix C++ library as well, and we could replace steps 2-3 with one that uses that implements the Region and Pixelization interfaces in a downstream C++ package. This would get this up and running faster, but I have heard rumblings that there are problems with its region intersections that the CDS Rust implementation does not have. More importantly, steps 2-3 are what makes this look fun to me. If this effort doesn't converge before having HEALPix in sphgeom becomes a higher priority for LSST, we can go back to trying to do this completely in C++. All the work in Step 1 carries over directly, and this design should even let someone have multiple HEALPix sphgeom implementations installed at the same time.
            Hide
            tjenness Tim Jenness added a comment -

            sphgeom on conda-forge might be easier if the library itself was distinct from the python bindings. My problem with building the python bindings with setuptools is that I haven't worked out how to make it build the C++ library and install it with the include files and then have the python bindings use it. Currently the C++ code is compiled directly into the python shared library.

            Show
            tjenness Tim Jenness added a comment - sphgeom on conda-forge might be easier if the library itself was distinct from the python bindings. My problem with building the python bindings with setuptools is that I haven't worked out how to make it build the C++ library and install it with the include files and then have the python bindings use it. Currently the C++ code is compiled directly into the python shared library.
            Hide
            jbosch Jim Bosch added a comment -

            As discussed at our last sprint-planning meeting, I'm handing this off to Matthias Wittgen , because as usual it turns out I don't have time for a "fun side project", and this is becoming a high priority, as the preferred way to get HEALPix support in the Gen3 butler.

            Here's where it's at:

            • As noted in the ticket description, we want to implement a new "HEALPixelization" subclass of lsst::sphgeom::Pixelization, backed by the third-party HEALPix library (ignore my first Jira comment about using a Rust implementation; let's consider that a dead end for now).  The hard part of this is providing a lsst::sphgeom::Region subclass to return in HEALPixelization::pixel, because HEALPix pixels are not bounded by great circles and hence we can't use lsst::sphgeom::ConvexPolygon.
            • The tickets/DM-13843 branch for sphgeom has a number of minor fixes, as well as two commits that start a HEALPixel intermediate (but still abstract) base class that is designed to be implemented by subclasses that live outside the sphgeom package itself.

            And here's what remains:

            • Create a new sphgeom_healpix package (follow the instructions at https://github.com/lsst/templates#readme for "stack package").
            • Set up its build system to compile and link against healpix_cxx from conda-forge;
            • Add pybind11 wrappers for HEALPixel to sphgeom itself (on the branch for this ticket).  Should be very little to do here, as you can just inherit bindings from lsst::sphgeom::Region.
            • Add a concrete subclass of HEALPixel to sphgeom_healpix.  Because non-great-circle spherical geometry is hard, but lsst::sphgeom::Region's relationships are carefully and intentionally defined to be conservative, I bet you can get away with implementing getBoundingCircle and delegating other relationship methods to that (e.g. declare that a HEALPixel is disjoint from a ConvexPolygon if the HEALPixel's bounding circle is disjoint from that ConvexPolygon).  But you may also find functions in healpix_cxx that do exactly what you need.  Update the HEALPixel base class in sphgeom as needed on the branch for this ticket.  This will mostly be reading the Region docs carefully and reading the healpix_cxx docs carefully and connecting the dots.
            • Use your concrete HEALPixel subclass to help implement a concrete HEALPixelization subclass of lsst::sphgeom::Pixelization in sphgeom_healpix.  I'm quite confident that you'll find healpix_cxx functions that do exactly what you need for most of the virtual methods here, but as before, finding them may be the hard work.
            • Add pybind11 wrappers for HEALPixelization to sphgeom_healpix; this will also serve as the Python factory for all HEALPixel instances, which should only need to be accessed in Python via the base class interface wrapped in sphgeom.
            • Add Python tests to sphgeom_healpix, using healpy as a reference implementation.
            • Talk to the #dm-arch folks about getting healpix_cxx into our standard conda environments.

            Since I'll probably be on leave during most of your work on this, please feel free to ask for help here or in #dm-science-pipelines Slack if you need it.  Two resources in particular:

            • You've already worked with Eli Rykoff.  He can probably help HEALPix mathematical definitions and healpy in particular.
            • Serge Monkewitz is the original author of sphgeom.  He's no longer with the project, but he's still a friend of the project (and has a Slack account) and is often willing to help with questions about spherical geometry in general and sphgeom itself specifically.

            I think the healpix_cxx docs can be found online here and/or here, but it'd be prudent to check those against version that we get from conda forge.  They look like the same thing, but the second link is 11 years older than the first one, and there may even be forks out in the wild.

            Show
            jbosch Jim Bosch added a comment - As discussed at our last sprint-planning meeting, I'm handing this off to Matthias Wittgen , because as usual it turns out I don't have time for a "fun side project", and this is becoming a high priority, as the preferred way to get HEALPix support in the Gen3 butler. Here's where it's at: As noted in the ticket description, we want to implement a new " HEALPixelization " subclass of lsst::sphgeom::Pixelization , backed by the third-party HEALPix library (ignore my first Jira comment about using a Rust implementation; let's consider that a dead end for now).  The hard part of this is providing a  lsst::sphgeom::Region subclass to return in HEALPixelization::pixel , because HEALPix pixels are not bounded by great circles and hence we can't use lsst::sphgeom::ConvexPolygon . The  tickets/DM-13843 branch for sphgeom has a number of minor fixes, as well as two commits that start a HEALPixel intermediate (but still abstract) base class that is designed to be implemented by subclasses that live outside the sphgeom package itself. And here's what remains: Create a new sphgeom_healpix package (follow the instructions at  https://github.com/lsst/templates#readme for "stack package"). Set up its build system to compile and link against healpix_cxx from conda-forge; Add pybind11 wrappers for HEALPixel to sphgeom itself (on the branch for this ticket).  Should be very little to do here, as you can just inherit bindings from lsst::sphgeom:: Region . Add a concrete subclass of HEALPixel to sphgeom_healpix .  Because non-great-circle spherical geometry is hard , but lsst::sphgeom::Region 's relationships are carefully and intentionally defined to be conservative, I bet you can get away with implementing getBoundingCircle and delegating other relationship methods to that (e.g. declare that a HEALPixel is disjoint from a ConvexPolygon if the HEALPixel's bounding circle is disjoint from that ConvexPolygon ).  But you may also find functions in healpix_cxx that do exactly what you need.  Update the HEALPixel base class in sphgeom as needed on the branch for this ticket.  This will mostly be reading the Region docs carefully and reading the healpix_cxx docs carefully and connecting the dots. Use your concrete  HEALPixel subclass to help implement a concrete HEALPixelization subclass of lsst::sphgeom::Pixelization in sphgeom_healpix .  I'm quite confident that you'll find healpix_cxx functions that do exactly what you need for most of the virtual methods here, but as before, finding them may be the hard work. Add pybind11 wrappers for HEALPixelization to sphgeom_healpix ; this will also serve as the Python factory for all HEALPixel instances, which should only need to be accessed in Python via the base class interface wrapped in sphgeom . Add Python tests to sphgeom_healpix , using healpy as a reference implementation. Talk to the #dm-arch folks about getting healpix_cxx into our standard conda environments. Since I'll probably be on leave during most of your work on this, please feel free to ask for help here or in #dm-science-pipelines Slack if you need it.  Two resources in particular: You've already worked with Eli Rykoff .  He can probably help HEALPix mathematical definitions and healpy in particular. Serge Monkewitz is the original author of sphgeom .  He's no longer with the project, but he's still a friend of the project (and has a Slack account) and is often willing to help with questions about spherical geometry in general and sphgeom itself specifically. I think the healpix_cxx docs can be found online here and/or here , but it'd be prudent to check those against version that we get from conda forge.  They look like the same thing, but the second link is 11 years older than the first one, and there may even be forks out in the wild.
            Hide
            tjenness Tim Jenness added a comment -

            Also bear in mind that if you are adding this new package as a butler dependency then we need some form of "pip install" support (see the hacks in sphgeom setup.py) because I really really don't want to lose the ability to test daf_butler in a github action. This may require that you have to fix the sphgeom setup.py to get it to properly install a standalone c++ library to link against rather than the current hack of ignoring a standalone library and just making a python interface. Doing the latter would make it properly pip installable and conda-forge-able (although in conda-forge land it might be better to have a C++ sphgeom and py-sphgeom packages separate). Anyway, I don't really care how it works so long as I don't end up breaking butler testing on github.

            Show
            tjenness Tim Jenness added a comment - Also bear in mind that if you are adding this new package as a butler dependency then we need some form of "pip install" support (see the hacks in sphgeom setup.py) because I really really don't want to lose the ability to test daf_butler in a github action. This may require that you have to fix the sphgeom setup.py to get it to properly install a standalone c++ library to link against rather than the current hack of ignoring a standalone library and just making a python interface. Doing the latter would make it properly pip installable and conda-forge-able (although in conda-forge land it might be better to have a C++ sphgeom and py-sphgeom packages separate). Anyway, I don't really care how it works so long as I don't end up breaking butler testing on github.
            Hide
            wittgen Matthias Wittgen added a comment -

            Show
            wittgen Matthias Wittgen added a comment -
            Hide
            wittgen Matthias Wittgen added a comment -

            Unassigned my name.
            Current status beyond Jim Bosch initial work
            DM-33162 fixes the underlying healpix_cxx compilation and provides a shared library now.

            Show
            wittgen Matthias Wittgen added a comment - Unassigned my name. Current status beyond Jim Bosch initial work DM-33162 fixes the underlying healpix_cxx compilation and provides a shared library now.
            Hide
            tjenness Tim Jenness added a comment -

            Is there any work on these unmerged branches that should be reviewed and merged given the current status of the work?

            Show
            tjenness Tim Jenness added a comment - Is there any work on these unmerged branches that should be reviewed and merged given the current status of the work?

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              jbosch Jim Bosch
              Watchers:
              Eric Rosas, Gregory Dubois-Felsmann, Jim Bosch, Matthias Wittgen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:

                  Jenkins Builds

                  No builds found.