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: interpolate: Use scipy.spatial.distance to speed-up Rbf #9222

Merged
merged 2 commits into from Sep 10, 2018

Conversation

@jrsassen
Copy link
Contributor

jrsassen commented Sep 4, 2018

The pairwise distances implemented in scipy.spatial are faster than the ones implemented in Rbf. Therefore, using them leads to a significant speed-up when using Rbf with a large number of points.

Previously a part of PR #9215

scipy/interpolate/rbf.py Outdated Show resolved Hide resolved
@jrsassen jrsassen force-pushed the jrsassen:spatial-distance branch 3 times, most recently from 1c0a39e to b9af092 Sep 10, 2018
@jrsassen

This comment has been minimized.

Copy link
Contributor Author

jrsassen commented Sep 10, 2018

By the way, this should be labeled scipy.interpolate instead of scipy.spatial, right?

Copy link
Member

larsoner left a comment

Other than one minor gripe, LGTM

scipy/interpolate/rbf.py Outdated Show resolved Hide resolved
@larsoner larsoner added this to the 1.2.0 milestone Sep 10, 2018
@jrsassen jrsassen force-pushed the jrsassen:spatial-distance branch from b9af092 to f908e2c Sep 10, 2018
The pairwise distances implemented in scipy.spatial are faster than the
ones implemented in Rbf. Therefore, using them leads to a significant
speed-up when using Rbf with a large number of points.
@jrsassen jrsassen force-pushed the jrsassen:spatial-distance branch from f908e2c to c0ad09c Sep 10, 2018
@jrsassen

This comment has been minimized.

Copy link
Contributor Author

jrsassen commented Sep 10, 2018

While at it, I fixed some other PEP8 violations. Hope it's ok I added this here.

Copy link
Member

larsoner left a comment

LGTM +1 for merge. I am fine with incrementally getting to PEP8 compliance by changing nearby lines.

Feel free to ping me at the end of the week for merge unless someone else comments.

@rgommers

This comment has been minimized.

Copy link
Member

rgommers commented Sep 10, 2018

Verified existing test coverage has the 1-D, 2-D, 3-D cases that should exercise the now removed code with np.newaxis. Looks good to me too now, merging. Thanks @jrsassen

@rgommers rgommers merged commit fce088d into scipy:master Sep 10, 2018
4 checks passed
4 checks passed
ci/circleci: build_docs Your tests passed on CircleCI!
Details
ci/circleci: pypy3 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.