Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Added jacobian computation to ComposeScaleSkewVersorTransform #1755

Merged
merged 2 commits into from
Apr 7, 2020
Merged

ENH: Added jacobian computation to ComposeScaleSkewVersorTransform #1755

merged 2 commits into from
Apr 7, 2020

Conversation

aylward
Copy link
Member

@aylward aylward commented Apr 7, 2020

Computed Jacobian using SymPy. See comments in code for computation
script.

Added test to verify Jacobian with finite difference results.

PR Checklist

  • No API changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
    Refer to the ITK Software Guide for
    further development details if necessary.

Computed Jacobian using SymPy.  See comments in code for computation
script.

Added test to verify Jacobian with finite difference results.
@aylward aylward requested review from blowekamp and thewtex April 7, 2020 17:42
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good way to do it.

@blowekamp
Copy link
Member

Very nice!

I recall doing something similar and thinking is was "too" complicated, but this looks simpler.

It will be interesting to see how it compare for registration with the additive composite transform.

COMP: Removed unused variables.  Resolved shadowed variable.
@dzenanz
Copy link
Member

dzenanz commented Apr 7, 2020

Preferably squash upon merge.

@thewtex
Copy link
Member

thewtex commented Apr 7, 2020

😎

@aylward aylward merged commit 2f38deb into InsightSoftwareConsortium:master Apr 7, 2020
@aylward aylward deleted the ComposeScaleSkewVersorJacobian branch April 7, 2020 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants