4
\$\begingroup\$

I created a generator function to iterate through all possible dice rolls, and then applied it the ability score generating methods in Pathfinder RPG. (Obviously, there are more efficient ways to approach probability outcome analysis from a number theoretic perspective, but this allows generic and quick analysis without too much mathematical understanding of the particular situation.)

My question is, using this iterator, is there any way to generically split the iterator into different numbers of dice? My current approach feels sloppy and is definitely not scalable to more varied numbers of dice. Obviously, I could just use separate dice iterators, but that would be a generally less efficient approach.

Also, as always, any other feedback is appreciated, even PEP 8 stylistic feedback.

math_supplement.py

from fractions import Fraction

def dice_outcomes(num, min=1, max=6):
    '''
    A generator function returning all outcomes of num dice as lists. Dice values are inclusively between min (default 1) and max (default 6).
    '''
    output = [min] * num
    try:
        while True:
            yield output
            i = 0
            while True:
                output[i] += 1
                if output[i] <= max:
                    break
                output[i] = min
                i += 1
    except IndexError:
        return

def sample_space_report(stats, size):
    '''
    Reports on stats relative to a sample space.

    stats
        a dict, with each key being an outcome and the 
    size
        the sample space
    '''
    probsum = 0
    for outcome, occurences in stats.items():
        prob = Fraction(occurences, size)
        probsum += prob
        print(f'{outcome}: {prob} ({float(prob):.2%})')
    print(f'Validation sum = {probsum}')

Pathfinder_ability_score_probability.py

from collections import Counter

from math_supplement import dice_outcomes, sample_space_report

class AbilityScoreStats:
    def __init__(self, roll_handler, sample_space):
        self.roll_handler = roll_handler
        self.sample_space = sample_space
        self.outcomes = Counter()

    def add(self, l):
        self.outcomes[self.roll_handler(l)] += 1


MIN = 1
MAX = 6
DICE_FACES = MAX - MIN + 1
ability_score_methods = {'Standard':
                             AbilityScoreStats(lambda rolls: sum(rolls) - min(rolls),
                             DICE_FACES ** 4),
                         'Classic':
                             AbilityScoreStats(lambda rolls: sum(rolls),
                             DICE_FACES ** 3),
                         'Heroic':
                             AbilityScoreStats(lambda rolls: sum(rolls) + 6,
                             DICE_FACES ** 2)
                        }
for rolls in dice_outcomes(2, min=MIN, max=MAX):
    ability_score_methods['Heroic'].add(rolls)
    for dice_three in range(1,7):
        working_rolls = rolls + [dice_three]
        ability_score_methods['Classic'].add(working_rolls)
        for dice_four in range(1,7):
            working_rolls = rolls + [dice_three, dice_four]
            ability_score_methods['Standard'].add(working_rolls)
for name, method in ability_score_methods.items():
    print(f'    {name}:')
    sample_space_report(method.outcomes, method.sample_space)
\$\endgroup\$
3
\$\begingroup\$

Your dice_outcomes function is not generic enough. It works for you with your use case but consider the following call:

list(dice_outcomes(3))

the result is surprising:

[[1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1], [1, 1, 1]]

This is because your yield output always return the same list; these are 216 references to the same list (bonus points if you are able to understand why this is always [min] * num). Using the rolls in a for loop as you do work because you transform the list each time and don't store a reference, but some use-case make your function buggy.

But all in all, this is not necessary to come with a fix (such as yield tuple(output)) because you are just reinventing itertools.product. You could instead write:

def dice_outcomes(num, min=1, max=6):
    yield from itertools.product(range(min, max+1), repeat=num)

Now about your class and for loops… the way you add dice rolls to already existing rolls feels really off. Especially given how you wrote a function to generate rolls from several die in the first place.

Besides, you can feed a generator to a Counter and it will happily count the occurences of each values:

self.outcomes = Counter(map(roll_handler, dice_outcomes(num, MIN, MAX)))

This means that:

  1. You don't need the add method;
  2. You would need to add num and possibly min and max as parameters;
  3. You can compute the sample_space in __init__.

Given how simple dice_outcomes is now and how tied the class and sample_space_report are, you could combine everything in this class:

from fractions import Fraction
from collections import Counter
from itertools import product


class AbilityScoreStats:
    def __init__(self, roll_handler, num_die, min_roll=1, max_roll=6):
        max_roll += 1  # Account for excluded upper bound and off-by-one substraction
        self.sample_space = (max_roll - min_roll) ** num_die
        self.outcomes = Counter(
            roll_handler(roll)
            for roll in product(range(min_roll, max_roll), repeat=num_die)
        )

    def print_report(self, title):
        print(f'    {title}:')

        prob_sum = 0
        for outcome, occurrences in self.outcomes.items():
            probability = Fraction(occurrences, sample_space)
            prob_sum += probability
            print(f'{outcome}: {probability} ({float(probability):.2%})')
        print(f'Validation sum = {prob_sum}')

Usage being:

standard = AbilityScoreStats(lambda roll: sum(roll) - min(roll), 4)
classic = AbilityScoreStats(sum, 3)
heroic = ability_score_stats(lambda roll: sum(roll) + 6, 2)

standard.print_report('Standard')
classic.print_report('Classic')
heroic.print_report('Heroic')

Now, there is still two issues:

  • The prob_sum variable validating the computed values shouldn't be left once you have tested your function;
  • A class not used as storage having only 2 functions, one of them is __init__ should be replaced by a function:
from fractions import Fraction
from collections import Counter
from itertools import product


def ability_score_stats(title, roll_handler, num_die, min_roll=1, max_roll=6):
    max_roll += 1  # Account for excluded upper bound and off-by-one substraction
    sample_space = (max_roll - min_roll) ** num_die
    outcomes = Counter(
        roll_handler(roll)
        for roll in product(range(min_roll, max_roll), repeat=num_die)
    )

    assert sample_space == sum(outcomes.values())

    print(f'    {title}:')
    for outcome, occurrences in outcomes.items():
        probability = Fraction(occurrences, sample_space)
        print(f'{outcome}: {probability} ({float(probability):.2%})')


if __name__ == '__main__':
    ability_score_stats('Standard', lambda roll: sum(roll) - min(roll), 4)
    ability_score_stats('Classic', sum, 3)
    ability_score_stats('Heroic', lambda roll: sum(roll) + 6, 2)

I somehow left the validation so you can still check that the computations are the same. But you can disable the check by running python -O your_script.py.

\$\endgroup\$
  • \$\begingroup\$ bonus points for figuring out why this is always [min] * num It always resets every digit to min as part of the carry before triggering the IndexError. I see... Given... how tied the class and sample_space_report are, you could combine everything in this class I could combine everything in one class, but I use sample_space_report in other modules. Is that a bad thing? A class not used as storage having only 2 functions, one of them is __init__ should be replaced by a function This seems like generically useful advice to keep in mind, is this generally true, or are there exceptions? \$\endgroup\$ – Graham Jul 29 '18 at 23:06
  • \$\begingroup\$ [continued] The prob_sum variable validating the computed values shouldn't be left once you have tested your function Why not? Won't that just be handled straightforwardly by the garbage collector once it goes out of scope? \$\endgroup\$ – Graham Jul 29 '18 at 23:06
  • 1
    \$\begingroup\$ @Graham If you use sample_space_report elsewhere too, this is reason enough to keep it separate. The prob_sum variable issue is not that it takes memory, as you said it will be gced once out of scope; rather that it serve as testing the function and is not really part of it. And for the class, yes this is generally true: stop writing classes \$\endgroup\$ – Mathias Ettinger Jul 30 '18 at 6:10

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.