-1

How can I avoid the nested for-loops in the following Code and using the map-function instead for example?

import numpy as np

A = np.array([[1,2,3],[4,5,6],[7,8,9],[10,11,12]])
n = len(A[0])
B = np.zeros((n,n))

for i in range(n):
    for j in range(n):
        B[j,i] = min(A[:,i]/A[:,j])
redwine_543
  • 19
  • 1
  • Your nested list is in effect a simple Cartesian product; `for i, j in itertools.product(range(n), repeat=2):` would give you the same indices without nesting. – Martijn Pieters Oct 27 '18 at 11:32

1 Answers1

1

I don't think map() is a good candidate for this problem because your desired result is nested rather than flattened. It'd be a little more verbose than just using a list comprehension to achieve your desired result. Here's a cleaner way of initializing B.

B = np.array([np.min(A.T/r, axis=1) for r in A.T])

This iterates over every column of A (every row of A.T) and computes the broadcasted division A.T/r. Numpy is able to optimize that far beyond what we can do with raw loops. Then the use of np.min() computes minimums of that matrix we just computed along every row (rows instead of columns because of the parameter axis=1) more quickly than we would be able to with the builtin min() because of internal vectorization.

If you did want to map something, @MartijnPieters points out that you could use itertools with itertools.product(range(len(A[0])), repeat=2) to achieve the same pairs of indices. Otherwise, you could use the generator ((r, s) for r in A.T for s in A.T) to get pairs of rows instead of pairs of indices. In either case you could apply map() across that iterable, but you'd still have to somehow nest the results to initalize B properly.

Note: that this probably isn't the result you would expect. The elements of A are integers by default (notice that you used, e.g., 3 instead of 3.). When you divide the integer elements of A you get integers again. In your code, that was partially obfuscated by the fact that np.zeros() casts those integers to floats, but the math was wrong regardless. To fix that, pass an additional argument when constructing A:

A = np.array([[1,2,3],[4,5,6],[7,8,9],[10,11,12]], float)
Hans Musgrave
  • 6,613
  • 1
  • 18
  • 37
  • `map` is still a built-in, and is an excellent tool for many use cases, and certainly not actively discouraged *unless you are building a list object where a list comprehension might make more sense*. – Martijn Pieters Oct 27 '18 at 11:29
  • `map` is still in the global namespace. It was `reduce` which was moved to `functools` in Python 3. – James Oct 27 '18 at 11:29
  • You appear to be confusing `map` for `reduce` here, which indeed moved to `functools` out of the built-in namespace. – Martijn Pieters Oct 27 '18 at 11:29
  • Oh, whoops. I'll fix that. Thank you. – Hans Musgrave Oct 27 '18 at 11:30
  • Thank you Hans, it works very well. However my aim was to improve performance and your answer certainly makes the code more readable, but does not improve performance. How can I rewrite it, that performance is improved? – redwine_543 Oct 27 '18 at 12:09
  • @redwine_543 You'll get better results from SO if you include details like that in the question, and SO loosely enforces the policy that followup questions should be asked separately. That said, your problem actually needs every one of those divisions computed, so the only way to speed it up is to speed the divisions up or reduce the looping overhead. Figuring out which one you need improved is heavily dependent on whether you're doing this problem a lot of times with small inputs (like your current size of `A`) or a few times with big inputs (thousands of elements in `A`). – Hans Musgrave Oct 27 '18 at 12:32
  • @redwine_543 For larger inputs, the code `B = np.array([np.min([r/s for r in A.T], axis=1) for s in A.T])` will offer some gains by speeding up the computation of minimums -- relying on numpy instead of the python builtin. – Hans Musgrave Oct 27 '18 at 12:33
  • @redwine_543 I came up with a much faster solution on my machine. I'll update my answer accordingly. – Hans Musgrave Oct 27 '18 at 12:54