3
\$\begingroup\$

I have two arrays, each with shape (1, 51, 150, 207)

I need to add them such that:

newarray[0][0][0][0] = array1[0][0][0][0] + array2[0][0][0][0]
newarray[0][0][0][1] = array1[0][0][0][1] + array2[0][0][0][1]

for every element in the array.

Currently I'm creating a new array using nested while loops but this approach is extremely time consuming. Can anyone offer a better solution?

H = 0
NS = 0
EW = 0
height = []
while H < 51:
    northsouth = []
    while NS < 150:
        eastwest = []
        while EW < 207:
            eastwest.append(PH[0][H][NS][EW] + PHB[0][H][NS][EW])
            EW += 1
            print eastwest
        northsouth.append(eastwest)
        NS += 1
        print northsouth
    height.append(northsouth)
    H =+ 1
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Arrays can definitely be tricky to work with, but I'm sure our Python pros will have some good advice for you! \$\endgroup\$ – Phrancis Jun 25 '15 at 23:10
  • \$\begingroup\$ How does this work? Have you tested it? \$\endgroup\$ – rolfl Jun 25 '15 at 23:13
  • \$\begingroup\$ So it starts by adding the [0][0][0][0] elements in the two arrays and appending them to a blank array. Once it goes through every element in the 4th position it increments up to [0][0][1][0] and does the same thing all the way through each index. \$\endgroup\$ – lthermin Jun 25 '15 at 23:41
  • \$\begingroup\$ The very last line, H =+ 1, shouldn't it be H += 1 ? \$\endgroup\$ – jperl Jun 26 '15 at 6:15
  • \$\begingroup\$ Why aren't you using numpy, in which this would be newarray = array1 + array2? \$\endgroup\$ – jonrsharpe Jun 26 '15 at 6:58
9
\$\begingroup\$

Firstly, code that looks like:

foo = 0
while foo < bar:
    ...  # foo doesn't change here
    foo += 1

is a bad sign. In Python, this can immediately be simplified, using range, to:

for foo in range(bar):
    ...

Secondly, hard-coding the shape of your "arrays" everywhere (having the literals 51, 150 and 207 in the code itself), referred to as "magic numbers", is poor practice in every programming language. These are the lengths of each sub-list, so should be calculated as needed using len(...). This makes the code more flexible; it can now be easily applied to add arrays of other sizes. it's also slightly awkward to hard-code the depth of the addition (i.e. that it only handles four-dimensional arrays).


Thirdly, however, code like:

for index in range(len(sequence)):
    element = sequence[index]
    ...

is generally better replaced with:

for element in sequence:
    ...

where you need the index too, you can use enumerate, and where you need to iterate over multiple iterables you can use zip; Python has lots of handy functions for dealing with iterables (see also itertools).


Fourthly, when you have a loop appending to a list, it's usually more efficient to replace it with a "list comprehension". For example:

foo = []
for bar in baz:
    foo.append(bar)

can be replaced by:

foo = [bar for bar in baz]

Finally, however, this is totally trivial if you switch to numpy, which is extremely helpful for manipulating arrays:

>>> import numpy as np
>>> arr1 = np.random.randint(1, 10, (1, 51, 150, 207))
>>> arr2 = np.random.randint(1, 10, (1, 51, 150, 207))
>>> arr1 + arr2  # wasn't that easy!
array([[[[11, 13, 14, ...,  9, 14,  8],
         [15,  4, 10, ..., 14, 10, 16],
         [11, 11,  9, ...,  4,  9,  9],
         ..., 
         [17, 10, 14, ..., 10, 12, 12],
         [12, 11, 14, ...,  7,  8,  8],
         [ 9, 11,  6, ...,  3, 11,  8]],

        [[10, 15, 16, ..., 11, 16, 10],
         [17, 17,  8, ...,  8,  7,  8],
         [11,  7,  2, ..., 16, 11,  5],
         ..., 
         [15, 10, 16, ..., 16, 13,  9],
         [15,  8,  7, ..., 13,  5,  6],
         [ 4,  5, 10, ...,  7, 10, 10]],

        [[11, 10,  4, ..., 10,  7, 11],
         [12,  3, 14, ..., 11, 12, 12],
         [ 6, 11, 16, ..., 14,  9, 12],
         ..., 
         [18, 10, 11, ..., 13, 14,  9],
         [11, 11, 10, ...,  9, 13, 12],
         [10, 10, 10, ..., 12, 14,  8]],

        ..., 
        [[16, 11, 12, ..., 13, 13,  9],
         [ 7, 15, 10, ...,  9, 11,  5],
         [ 5, 11, 14, ..., 14,  4, 11],
         ..., 
         [15, 13,  6, ..., 17,  6, 10],
         [ 9, 12,  7, ...,  7, 17, 11],
         [12,  9, 10, ...,  4,  9,  5]],

        [[15, 12, 10, ...,  5, 12, 15],
         [17, 14, 15, ..., 13, 11,  8],
         [13, 10, 10, ...,  8,  4,  8],
         ..., 
         [12,  9, 10, ...,  9,  7, 12],
         [13, 12, 17, ...,  7, 13, 10],
         [13,  6,  8, ..., 15, 13,  7]],

        [[ 9,  8,  8, ...,  6, 10,  2],
         [ 9, 15, 14, ..., 14,  4,  5],
         [ 3, 12, 12, ...,  5, 10, 14],
         ..., 
         [ 6, 11, 15, ..., 11,  4, 15],
         [ 4, 12, 14, ..., 12, 11,  9],
         [10,  3,  5, ...,  5,  7, 10]]]])
\$\endgroup\$
1
\$\begingroup\$

One thing I can always recommend is to move redundant code into the outermost scope possible. When you are calling PH[0][H][NS][EW], you might as well be doing this:

note: This code is not analogous to yours, it's simply for demonstration purposes

while H < 51
    while NS < 150
        while EW < 207
            # everything below is executed 1,583,550 times (51*150*207)
            root_array = PH[0]   # 1.5 million times for this one
            H_array = root_array[H] # 1.5 million times for this one too
            NS_array = H_array[NS]  # now we're at over 4.5 million evaluations
            # the previous 3 evaluations are unnecessary in this scope

            EW_value = NS_array[EW]
            # NOW do something with EW_value

The core loop (while EW < 207) does not need to re-evaluate all of those array expressions. By moving the values to the outermost scope possible, you minimize inefficient redundancy.

Suggestion:

root_array = PH[0]                   # PH[0] is now only evaluated once
while H < 51
    H_array = root_array[H]          # only evaluated 51 times (saves 1,583,499 cycles)
    while NS < 150
        NS_array = H_array[NS]       # evaluated 7,650 times (saves 1,575,900 cycles)
        while EW < 207
            EW_value = NS_array[EW]
            # do something with EW_value

Using this method we have prevented the unnecessary execution of 4,742,949 reads

\$\endgroup\$

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.