2
\$\begingroup\$

This takes two sets of five random points stored as a NumPy matrix, and then calculates the NumPy matrix between a point of the first set and a point of the second set. While the code works, I feel it's inelegant, as if I was writing in PHP or some other language. How can the function CalculateMatrixDistances be improved?

import random
import numpy as np
import scipy
from scipy import spatial
P=5
V=5
def CalculateMatrixDistances(MatRows,MatColums):
 numberRows=MatRows.shape[0]
 numberColumns=MatColums.shape[0]
 MatrixDistances=np.matrix( [0 for c in range(numberColumns)] )
 for r in range(numberRows):
 d=[scipy.spatial.distance.euclidean (MatRows[r],MatColums[c]) for c in range(numberColumns)]
 MatrixDistances = np.vstack([MatrixDistances, d])
 MatrixDistances=np.delete(MatrixDistances, (0), axis=0)
 return MatrixDistances
PositionP=np.random.rand(P,2)
PositionV=np.random.rand(V,2)
MatrixDistancesVP=CalculateMatrixDistances(PositionV,PositionP)
print PositionP
print PositionV
print MatrixDistancesVP
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Feb 29, 2016 at 15:30
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

(This is already implemented as scipy.spatial.distance.cdist. I'm going to assume that it's more optimised too.)

  • Why initialise MatrixDistances to zero if you're only going to delete the first column regardless? A matrix can also have zero for any dimension.

Some more general observations:

  • As always, take a look at PEP8 for Python conventions, in particular naming of functions and variables, but also missing whitespace.
  • The imports could be more consistent; the import spatial isn't being used.
  • A zero matrix should probably be allocated with numpy.matlib.zeros since it's shorter.
  • In Python 2 use xrange if you don't need the full list; also use the function form of print for compatibility with Python 3 (it's also more consistent to use it this way).
  • The repeated vstack may not be the most efficient option, but you should consult a profiler for that.

When keeping the naming it should still rather look like this maybe.

import random
import numpy as np
import numpy.matlib
import scipy
import scipy.spatial
P, V = 5, 5
def CalculateMatrixDistances(MatRows, MatColums):
 numberRows = MatRows.shape[0]
 numberColumns = MatColums.shape[0]
 MatrixDistances = np.matlib.zeros((0, 5))
 for r in xrange(numberRows):
 d = [scipy.spatial.distance.euclidean(MatRows[r], MatColums[c]) for c in xrange(numberColumns)]
 MatrixDistances = np.vstack([MatrixDistances, d])
 return MatrixDistances
PositionP = np.random.rand(P, 2)
PositionV = np.random.rand(V, 2)
MatrixDistancesVP = CalculateMatrixDistances(PositionV, PositionP)
print(PositionP)
print(PositionV)
print(MatrixDistancesVP)
print(scipy.spatial.distance.cdist(PositionV, PositionP))
answered Mar 1, 2016 at 0:15
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.