Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

It has been a very long time since I've used Python.

What I'm looking for: I would like to create a 6-by-6 random matrix where each component is either 1 or 2, so that there are 18 ones and 18 twos.

Attempt:

import numpy
import random

array = numpy.zeros((6, 6))
while (array == 1).sum() != 18 or (array == 2).sum() != 18: 
    for i in range(0, 6):
        for j in range(0, 6):
            array[i][j] = random.randint(1, 2) 
print array    

Sure, this works. But this seems to me to be extremely inefficient, as the code is literally having to insert new matrix values until the number of ones/twos is 18. How can I make this more efficient?

share|improve this question

2 Answers 2

up vote 11 down vote accepted

Just create a list of 18 ones and 18 twos, shuffle it, then reshape to 6x6:

from random import shuffle
from numpy import reshape

nums = [1]*18 + [2]*18
shuffle(nums)
arr = reshape(nums, (6, 6))

Produces (for example):

array([[1, 2, 2, 1, 2, 1],
       [2, 2, 2, 2, 2, 1],
       [2, 2, 2, 1, 1, 2],
       [1, 1, 1, 1, 2, 2],
       [2, 1, 1, 2, 1, 1],
       [1, 2, 1, 1, 1, 2]])
share|improve this answer

tzaman's approach is much better and I can't top it, but I can point out another way that your approach could be improved. You have a while loop that would create an entirely new array every time there was any value off, because it loops through every single value and replaces it in your nested for loop.

At the very least you should check which there's too much of and not replace values that you have too little of. Something like this:

while (array == 1).sum() != 18 or (array == 2).sum() != 18: 
    reduce_ones = (array == 1).sum() > 18
    for i in range(0, 6):
        for j in range(0, 6):
            if ((array[i][j] == 1 and reduce_ones):
                  or (array[i][j] == 2 and not reduce_ones)):
                array[i][j] = random.randint(1, 2) 

The poorly named reduce_ones boolean will determine if there's too many 1s, and if it's False there must be too many 2s. Then for each iteration you check which value the current element is as well as whether that's the amount you have too much of. That way you only re-assign values that are in the majority and skip ones that you need more of.

When making algorithms like this you shouldn't throw away work, that's what will lose you a lot of time especially when it's not that hard to preserve the work with a bit more engineering in the algorithm.

share|improve this answer
    
I'm not sure this would actually converge any faster. Assuming you got close to 50/50 the first time, the second pass would flip around half of your "too few" number, putting you closer to 75/25. The next iteration after that would flip around half of the 75, which puts you at ~38/62, and so on. At least the original approach is trying for 50/50 each time. –  tzaman 13 hours ago
    
(cont.. ) In general, if you're targeting an exact amount of something, it's usually better to generate that directly if possible. One way here would be to get all the indices of the "too many" element, then use random.sample to pick exactly enough of those to get you to 18/18 and flip only those. Then again, you could also just do that for the initial generation (i.e. random.sample to get 18 indices of your 6x6, and set those to 1 and the rest to 2). –  tzaman 13 hours ago
    
@tzaman You're completely right! I just ran this through timeit, and the original approach is more than 4 times faster than my 'clever' one. I should have benchmarked it, especially because (as your say) there's a 50/50 chance of getting a valid list on each iteration. –  SuperBiasedMan 12 hours ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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