-1

I'm trying to generate about 6000 random images using python's pillow library. Right now I can't create more than 300 at a time or my program hits the maximum recursion depth. I understand that this is happening because my recursion isn't called with an "n-1" case. Being that all the images are selected using random numbers, I'm not sure how to address this issue. Here is my code:

## Generate Traits

TOTAL_IMAGES = 300 # Number of random unique images we want to generate

all_images = [] 

# A recursive function to generate unique image combinations
def create_new_image():
    
    new_image = {} 

    # For each trait category, select a random trait based on the weightings 
    new_image ["Plant"] = random.choices(plant, plant_weights)[0]
    new_image ["Pot"] = random.choices(pot, pot_weights)[0]
    new_image ["Face"] = random.choices(face, face_weights)[0]
    
    if new_image in all_images:
        return create_new_image()
    else:
        return new_image
    
    
# Generate the unique combinations based on trait weightings
for i in range(TOTAL_IMAGES): 
    
    new_trait_image = create_new_image()
    
    all_images.append(new_trait_image)
Rob
  • 7
  • 2
  • 8
    Perhaps you should use some other form of iteration instead of recursion. – quamrana Aug 24 '21 at 15:06
  • 3
    Python doesn't do TCO (tail call optimization) so trying to implement an iterative loop as recursion frequently leads to recursion depth errors. Use a `while` loop instead. I'd also suggest that if your possibility set is small enough, rather than doing guess-and-test (which apparently takes a LOT of guessing if you're maxing the recursion depth) you use `itertools` to generate an exhaustive list and `random.shuffle` it. – Samwise Aug 24 '21 at 15:07
  • 2
    how many plant pot and face items are there? – JonSG Aug 24 '21 at 15:07
  • 1
    Good question @JonSG. It wouldn't surprise me if there are actually fewer than 300 combinations and so it's recursing infinitely. – Samwise Aug 24 '21 at 15:09
  • 1
    Great example of [why not to use recursion in Python](https://stackoverflow.com/questions/66842109/recursion-applied-to-lists-vs-trees-in-python/66846438#66846438). Even if the possible combos of pots and plants is more than 300, it's still an abuse of recursion -- you can hit an unlucky run of randoms and blow the stack for no reason. – ggorlen Aug 24 '21 at 15:09
  • @JonSG there are 7 of each plant, pot and face. In the future I plan on including 5 more "attributes" as well – Rob Aug 24 '21 at 15:15
  • @ggorlen this is a good point i hadn't thought of. perhaps I should just try creating all 6000 in sperate batches? – Rob Aug 24 '21 at 15:19
  • 1
    The weights complicate pre-computing all possibilities a bit. Instead of 3 independent weighted choices, you have `7**3` choices, with up to `7**3` associated weights. – chepner Aug 24 '21 at 15:20
  • 343 combinations isn't too bad; 7**8 is getting a bit iffy. A quick experiment showed that that in 20 trials, the number of picks from 5.7 million possibilities need to get 3000 unique elements was between 3000 and 3002. I.e., you probably don't have to worry about duplicates. – chepner Aug 24 '21 at 15:26

2 Answers2

0

Perhaps you could just use a while True: loop to replace the recursion:

def create_new_image():
    while True:  
        new_image = {} 
        
        # For each trait category, select a random trait based on the weightings 
        new_image ["Plant"] = random.choices(plant, plant_weights)[0]
        new_image ["Pot"] = random.choices(pot, pot_weights)[0]
        new_image ["Face"] = random.choices(face, face_weights)[0]
    
        if new_image not in all_images:
            return new_image
quamrana
  • 37,849
  • 12
  • 53
  • 71
  • 2
    It might be worth using a `for` loop and adding some large number as a default parameter, say, `tries=10**7`, then raising an error if these tries are exhausted. – ggorlen Aug 24 '21 at 15:12
  • Wouldn't switching to loops instead of recursion greatly increase my time complexity? – Rob Aug 24 '21 at 15:20
  • How do you figure that? The TC is the same. – ggorlen Aug 24 '21 at 15:21
  • 2
    Iteration is iteration whether its recursion or not. No effect on time complexity. – quamrana Aug 24 '21 at 15:21
  • 1
    @ggorlen I'd prefer it to keep going and just print "I can do this all day" every few seconds. – Stefan Pochmann Aug 24 '21 at 15:23
  • @StefanPochmann I guess better yet is computing whether it's possible to fulfill the random requirements eventually, then raising if it's not. – ggorlen Aug 24 '21 at 15:26
  • See NASA Coding Rule #2 : https://www.reddit.com/r/learnprogramming/comments/4npim9/10_coding_practices_which_allows_nasa_to_write/ – JonSG Aug 24 '21 at 18:35
0

I found the error in my thinking. The recursion DOES work as it should. Because I only had 7^3 possibilities at the moment (due to not having the other attributes finished) my recursion was simply maxing out needlessly. Once I included the future attributes my program had no issue cranking out 6000 unique possibilities because there are 7^8 to choose from.

Thanks to @chepner for the tip off in the comments

Rob
  • 7
  • 2