Skip to main content
Code Review

Return to Answer

deleted 1 character in body
Source Link
Graipher
  • 41.6k
  • 7
  • 70
  • 134

If you were using python 2.x, there weouldwould be no need to do doors = [i for i in range(1, num_doors+1)], because range just returns a list. So just do:

If you were using python 2.x, there weould be no need to do doors = [i for i in range(1, num_doors+1)], because range just returns a list. So just do:

If you were using python 2.x, there would be no need to do doors = [i for i in range(1, num_doors+1)], because range just returns a list. So just do:

added 11 characters in body
Source Link
Graipher
  • 41.6k
  • 7
  • 70
  • 134

If you arewere using python 2.x, there isweould be no need to do doors = [i for i in range(1, num_doors+1)], because range just returns a list. So just do:

IfSince you are using python 3.x, range will return a range object, but you can just call list on it:

If you are using python 2.x, there is no need to do doors = [i for i in range(1, num_doors+1)], because range just returns a list. So just do:

If you are using python 3.x, range will return a range object, but you can just call list on it:

If you were using python 2.x, there weould be no need to do doors = [i for i in range(1, num_doors+1)], because range just returns a list. So just do:

Since you are using python 3.x, range will return a range object, but you can just call list on it:

added 2478 characters in body
Source Link
Graipher
  • 41.6k
  • 7
  • 70
  • 134

Minor last point: there is no need for the intermediate variable probability, just return successes / trials.


Python has an official style-guide, PEP8, which recommends not having unnecessary blank lines (such as between your if and else block).

Also, you should add one space in front between # and the actual comment.

It is also customary to use _ as a loop variable that is unused.


You can make your code slightly more efficient by only drawing the user's switched choice when actually switching:

def monty_hall (num_doors, trials, switch):
 successes = 0
 for _ in range(trials):
 # determine behind which door the prize is
 doors = list(range(1, num_doors+1))
 prize_door = np.random.choice(doors)
 
 # door the guest chooses. (Fixed to 1)
 guest_choice = 1
 doors.remove(guest_choice)
 # determine which door the host opens before guest is allowed to switch
 # this will be faster for larger lists than copying the `doors` list
 host_choice = prize_door
 while host_choice == prize_door:
 host_choice = np.random.choice(doors)
 doors.remove(host_choice)
 # determine which door the guest chooses if switching
 if switch:
 guest_choice = np.random.choice(doors)
 # check win condition
 if guest_choice == prize_door:
 successes += 1
 return successes / trials

It might actually be faster (for larger number of doors it will be), not to copy the doors for the host choice, but just take a random door from doors and take another one if it is the prize_door:

host_choice = prize_door
while host_choice == prize_door:
 host_choice = np.random.choice(doors)
doors.remove(host_choice)

For small lists this will be better:

host_doors = doors[:]
host_doors.remove(prize_door)
host_choice = np.random.choice(host_doors)
doors.remove(host_choice)

Overall the most costly part of this code is probably the removes, which can be quite costly for large lists if removing near the beginning (like for guest_choice). It would be better to store the doors as a set (for which deleting is O(1)). But that makes choosing one door harder (you have to do np.random.choice(tuple(doors)) everytime you draw). You should time both approaches and see what works best for your data.

Minor last point: there is no need for the intermediate variable probability, just return successes / trials.

Minor point: there is no need for the intermediate variable probability, just return successes / trials.


Python has an official style-guide, PEP8, which recommends not having unnecessary blank lines (such as between your if and else block).

Also, you should add one space in front between # and the actual comment.

It is also customary to use _ as a loop variable that is unused.


You can make your code slightly more efficient by only drawing the user's switched choice when actually switching:

def monty_hall (num_doors, trials, switch):
 successes = 0
 for _ in range(trials):
 # determine behind which door the prize is
 doors = list(range(1, num_doors+1))
 prize_door = np.random.choice(doors)
 
 # door the guest chooses. (Fixed to 1)
 guest_choice = 1
 doors.remove(guest_choice)
 # determine which door the host opens before guest is allowed to switch
 # this will be faster for larger lists than copying the `doors` list
 host_choice = prize_door
 while host_choice == prize_door:
 host_choice = np.random.choice(doors)
 doors.remove(host_choice)
 # determine which door the guest chooses if switching
 if switch:
 guest_choice = np.random.choice(doors)
 # check win condition
 if guest_choice == prize_door:
 successes += 1
 return successes / trials

It might actually be faster (for larger number of doors it will be), not to copy the doors for the host choice, but just take a random door from doors and take another one if it is the prize_door:

host_choice = prize_door
while host_choice == prize_door:
 host_choice = np.random.choice(doors)
doors.remove(host_choice)

For small lists this will be better:

host_doors = doors[:]
host_doors.remove(prize_door)
host_choice = np.random.choice(host_doors)
doors.remove(host_choice)

Overall the most costly part of this code is probably the removes, which can be quite costly for large lists if removing near the beginning (like for guest_choice). It would be better to store the doors as a set (for which deleting is O(1)). But that makes choosing one door harder (you have to do np.random.choice(tuple(doors)) everytime you draw). You should time both approaches and see what works best for your data.

deleted 388 characters in body
Source Link
Graipher
  • 41.6k
  • 7
  • 70
  • 134
Loading
added 131 characters in body
Source Link
Graipher
  • 41.6k
  • 7
  • 70
  • 134
Loading
Source Link
Graipher
  • 41.6k
  • 7
  • 70
  • 134
Loading
lang-py

AltStyle によって変換されたページ (->オリジナル) /