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

Allow Transform to return a matrix of derivatives

    Details

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

      Description

      The Transform class being introduced in DM-8439 does not provide any measure of how quickly destination-frame coordinates change as a function of source-frame coordinates. A getJacobian method should provide a matrix with these derivatives.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment - - edited

            One significant suggestion not mentioned on github is to add a pure Python wrapper around the pybind11 getJacobian wrapper that forces the result of getJacobian to always be two dimensional by setting the shape attribute to the expected value. It is unfortunate that pybind11/ndarray doesn't handle this correctly, but I am not convinced we should put up with the automatic squeezing of the returned array.

            Other than that and a few minor suggestions on github, it looks good to me.

            Jim Bosch do you have any idea what might be required for the pybind11 wrapper to return an array with all dimensions intact?

            Show
            rowen Russell Owen added a comment - - edited One significant suggestion not mentioned on github is to add a pure Python wrapper around the pybind11 getJacobian wrapper that forces the result of getJacobian to always be two dimensional by setting the shape attribute to the expected value. It is unfortunate that pybind11/ndarray doesn't handle this correctly, but I am not convinced we should put up with the automatic squeezing of the returned array. Other than that and a few minor suggestions on github, it looks good to me. Jim Bosch do you have any idea what might be required for the pybind11 wrapper to return an array with all dimensions intact?
            Hide
            jbosch Jim Bosch added a comment -

            I don't think there's much we can do locally that's better than continueClass with an explicit reshape in the Python layer (or the equivalent in a C++ lambda that uses lots of pybind11::object::attr calls).

            As discussed with Krzysztof Findeisen recently on Slack, we could switch globally to using numpy.matrix for Eigen objects in Python, but I'm not sure we want to do it in just one place.

            Show
            jbosch Jim Bosch added a comment - I don't think there's much we can do locally that's better than continueClass with an explicit reshape in the Python layer (or the equivalent in a C++ lambda that uses lots of pybind11::object::attr calls). As discussed with Krzysztof Findeisen recently on Slack, we could switch globally to using numpy.matrix for Eigen objects in Python, but I'm not sure we want to do it in just one place.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Actually, if numpy.matrix really does have inheritance-related bugs and other user-unfriendliness in it (as the link you sent me implies), then I'm not keen to adopt it.

            Show
            krzys Krzysztof Findeisen added a comment - Actually, if numpy.matrix really does have inheritance-related bugs and other user-unfriendliness in it (as the link you sent me implies), then I'm not keen to adopt it.
            Hide
            jbosch Jim Bosch added a comment -

            Fair enough. I actually didn't read that link in as much depth as you did; your summary scares me off, too.

            Show
            jbosch Jim Bosch added a comment - Fair enough. I actually didn't read that link in as much depth as you did; your summary scares me off, too.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Merged with all requested changes.

            Show
            krzys Krzysztof Findeisen added a comment - Merged with all requested changes.

              People

              • Assignee:
                krzys Krzysztof Findeisen
                Reporter:
                krzys Krzysztof Findeisen
                Reviewers:
                Russell Owen
                Watchers:
                Jim Bosch, Krzysztof Findeisen, Russell Owen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel