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

Use pybind11's native Eigen wrapping instead of ndarray EigenView

    XMLWordPrintable

    Details

      Description

      Update our pybind11 wrappers as needed to use pybind11's native Eigen support. This is necessary in order to upgrade to Eigen 3.3.

      Changes include:

      • Build ndarray without EigenView support and with native pybind11 Eigen support. The latter is not necessary but avoids the need to change all our wrappers that wrap code that uses Eigen to explicitly import pybind11/eigen.h
      • Update code and tests and needed. For example geom has one failing test because pybind11's Eigen wrappers are more lenient than ndarray, so it is now possible to construct an lsst.geom.Extent2I from an lsst.geom.Extent2D.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment - - edited

            I also found and fixed a few instances where I had missed calls to ndarray::Array::asEigen

            Note that ndarray can be built in such a way that it includes pybind11/eigen.h in its own ndarray/eigen.h file, but I chose to do that. I felt it's better to explicitly include the former in wrappers that require it, because that is the standard pybind11 way to do things and because not all objects that use eigen necessarily also use ndarray.

            Also note that I withdrew the pull request for meas_algorithms because the changes were for DM-14842 rather than this ticket.

            Show
            rowen Russell Owen added a comment - - edited I also found and fixed a few instances where I had missed calls to ndarray::Array::asEigen Note that ndarray can be built in such a way that it includes pybind11/eigen.h in its own ndarray/eigen.h file, but I chose to do that. I felt it's better to explicitly include the former in wrappers that require it, because that is the standard pybind11 way to do things and because not all objects that use eigen necessarily also use ndarray. Also note that I withdrew the pull request for meas_algorithms because the changes were for DM-14842 rather than this ticket.
            Hide
            Parejkoj John Parejko added a comment -

            Only one comment on a PR, otherwise these look good to merge.

            Show
            Parejkoj John Parejko added a comment - Only one comment on a PR, otherwise these look good to merge.

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              John Parejko
              Watchers:
              Jim Bosch, John Parejko, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: