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

Performance improvements for Similarity family #397

Merged
merged 3 commits into from
Jul 17, 2014

Conversation

jabooth
Copy link
Member

@jabooth jabooth commented Jul 16, 2014

Simple improvements to Similarity transform family gives some nice speed ups to Procrustes.

print('{} {}D pointclouds each of {} '
      'points'.format(len(ps), ps[0].n_dims, 
      ps[0].n_points))
88 3D pointclouds each of 60 points

%timeit GeneralizedProcrustesAnalysis(ps)
296 ms -> 119 ms

%timeit AlignmentSimilarity(ps[0], ps[1])
833 µs -> 317 µs

@patricksnape
Copy link
Contributor

These are failing 😒

@jabooth
Copy link
Member Author

jabooth commented Jul 17, 2014

Ah this test was not running on my machine due to numerical instabilities between OS X and Linux. Investigating.

@jabooth
Copy link
Member Author

jabooth commented Jul 17, 2014

updated the test suite to pass with the new numbers.

To be honest, the test that failed is highly numerically unstable - it's only useful in flagging up the fact that something numerically unstable has changed in the code that the SDM fitter uses, which is exactly what has happened here.

This isn't surprising, as for efficiency reasons this PR moves from doing

b.apply(a.apply(x))

to

a.compose_before(b).apply(x)

which will lead to slight numerical differences that aren't 'good' or 'bad'.

@patricksnape
Copy link
Contributor

+1 fair explanation

jabooth added a commit that referenced this pull request Jul 17, 2014
Performance improvements for Similarity family
@jabooth jabooth merged commit 0e2be0a into menpo:master Jul 17, 2014
@jabooth jabooth deleted the fastproc branch July 17, 2014 14:36
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.

None yet

2 participants