5
\$\begingroup\$

I need to create matrices that follow a specific format: the matrix is a square NxN matrix and the off-diagonals all have the same value (param) and the diagonals have values such that the rows sum to zero (in other words, they are equal to 0 - param * (N-1). The output is a numpy array

Here is my code. Is there any way I can make this more readable/elegant? Speed is also important.

import numpy as np

def makeMatrix(param, N):
    ar = []
    for i in range(N):
        ar.append([ param for st in range(N) ])
        ar[i][i] = 0-(param*(N-1))

    return np.array(ar, dtype=np.double)
\$\endgroup\$
5
\$\begingroup\$

One of the main features of NumPy is that you can work with matrices without explicit looping. To get the diagonal, use np.identity(). To set everything else, use broadcasting.

def make_matrix(value, dim):
    return value - value * dim * np.identity(dim, dtype=np.double)

param and N don't mean anything to me. I suggest value and dim. The function name should preferably be make_matrix, according to PEP 8.

\$\endgroup\$
3
\$\begingroup\$

You're calculating the diagonal value every time even though it's always the same. Just calculate it once before the loop.

diagonal = -param * (N - 1)

It's a little bit faster to make a list comprehension than loop and append to a list, even if you still need to loop over the list afterwards to set the values

ar = [[param for _ in range(N)]
      for i in range(N)]
for i in range(N):
    ar[i][i] = diagonal

You can fold in the diagonal assignment there, it will be harder to read but a little bit faster.

ar = [[param if i != j else diagonal
       for i in range(N)]
      for j in range(N)]

It's using a ternary expression, in simpler terms those look like a if test else b, where the value is set as a if test evaluates as True, and b if it evaluates as False. So this means that you just set the value as param when it's not a diagonal, and it's set as diagonal on the diagonals.

\$\endgroup\$
2
\$\begingroup\$

You almost certainly want to preallocate the matrix at the correct, final size and then assign values other than the default. Also look at the array creation routines for predefined functions.

E.g. you could use full to fill with param, then use the loop for the diagonal.

\$\endgroup\$
2
\$\begingroup\$

Style review

From a style point of view, you are violating PEP8, and should name your variables somewhat differently. Maybe something like:

  • makeMatrix into snakecase make_matrix or fill_diagonal_matrix. make_matrix is kind of anonymous and doesn't convey what the function does
  • ar into my_array
  • st into _, as you don't seem to use it
  • param into default_value
  • N into either n or matrix_size

And if using Python 2.x you should use xrange(n) as it doesn't generate the list, but uses a generator, which in turn will save you some memory and gain a little performance for large \$n\$'s.

Code review

You don't need a list comprehension to initialise all the off-diagonals values, you simply multiply it. With new variable names, and a docstring your function could look like:

 def fill_diagonal_matrix(default_value, matrix_size):
    """Fill matrix with default_value, except a zeroing diagonal.

    That is all elements are equal to default_value, except down
    the diagonal which equals to: - default_value * (matrix_size - 1)
    """
    my_array = []
    for i in xrange(matrix_size):
        my_array.append([default_value] * matrix_size)
        my_array[i][i] = -default_value * (matrix_size - 1)
    return my_array
\$\endgroup\$
-1
\$\begingroup\$

Question is, do you really need an explicit matrix? For large \$N\$ it occupies a significant space while containing very little information. I'd go for a (wrapped) function instead:

    def make_special_matrix(N, param):
        def special_matrix(i, j):
            if i == j:
                return (N-1)*param
            return param
        return special_matrix
\$\endgroup\$
  • \$\begingroup\$ N is never going to be very large for me, and it does need to be in the form of a numpy array because I feed it into some Cython code that requires a numpy array \$\endgroup\$ – C_Z_ Oct 8 '15 at 23:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.