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\$
5
  • \$\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
    Commented Jun 25, 2015 at 23:10
  • \$\begingroup\$ How does this work? Have you tested it? \$\endgroup\$
    – rolfl
    Commented Jun 25, 2015 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
    Commented Jun 25, 2015 at 23:41
  • \$\begingroup\$ The very last line, H =+ 1, shouldn't it be H += 1 ? \$\endgroup\$
    – jperl
    Commented Jun 26, 2015 at 6:15
  • \$\begingroup\$ Why aren't you using numpy, in which this would be newarray = array1 + array2? \$\endgroup\$
    – jonrsharpe
    Commented Jun 26, 2015 at 6:58

2 Answers 2

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\$
0
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\$
0

Your Answer

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.