5
\$\begingroup\$

I'm trying to write a Poker hand evaluator using NumPy, because I find pure Python is pretty slow.

def eval_(*args):

    if len(args) == 5: 
        hand = np.asarray([*args], dtype=np.int8)
    else:
        hand = np.asarray(args[0], dtype=np.int8)

    ranks = np.bincount(hand // 4, minlength = 13)  
    count = np.array([np.where(ranks == count)[0] for count in range(5)])

    if len(count[4]):
        return 8, count[4][0], count[1][0]
    if len(count[3]) and len(count[2]):
        return 7, count[3][0], count[2][0]
    if len(count[3]):
        return 4, count[3][0], count[1][1], count[1][0]
    if len(count[2]) == 2:
        return 3, count[2][1], count[2][0], count[1][0]
    if len(count[2]):
        return 2, count[2][0], count[1][2], count[1][1], count[1][0]

    is_straight = count[1][0] + 4 == count[1][4]
    is_acestraight = count[1][4] - 9 == count[1][3]
    is_flush = 5 in np.bincount(hand % 4)
    if is_straight: return 5 + 4 * is_flush, count[1][4]
    if is_acestraight: return 5 + 4 * is_flush, 3

    return 1 + 5 * is_flush, count[1][4], count[1][3], count[1][2], count[1][1], count[1][0]

I'm not satisfied with my code. I don't like all kinds of count[1][2] calls, I should be able to just attach the whole count[1] array to my return value but I don't know how to do it. And I have trouble with np.where(). I don't think I should use list comprehension for different count values on this:

count = np.array([np.where(ranks == count)[0] for count in range(5)])

but I have no idea.

\$\endgroup\$
  • 1
    \$\begingroup\$ Assuming count is a 2d array, it is better form to index it as count[3,0] or count[1,:]. But maybe it would better to leave it as a nested list. Except for the use of bincount I doubt if the use of arrays will give any speed improvement. \$\endgroup\$ – hpaulj Aug 10 '17 at 4:19
1
\$\begingroup\$

I find [interpreted] pure python is pretty slow.

Yes, that's true. And numpy operations can offer speed advantages, since its loops are coded in compiled C. But that's only significant when manipulating large arrays, and 5 seems like a pretty small number, too small to realize gains. (I think eval_() only evaluates a single hand at a time, it's not like you're passing in thousands of hands in a single call.)

Yes, you could return count[1] instead of count[1][2], but that's quite a different thing, presenting callers with a different API. Would you please state your concern in terms of what values the caller should receive, similar to a unit test?

The list comprehension you use seems pretty reasonable to me.

You have lots and lots of magic numbers that you might define identifiers for.

It wouldn't hurt to mention https://en.wikipedia.org/wiki/List_of_poker_hands in a comment, if that matches the rankings you're returning.

\$\endgroup\$
1
\$\begingroup\$

You could have given us more info such as to how you represent the cards. It would be better if you had clear separate functions that break the cards by their suit or face value. Probably even better to use object oriented programming with some class containing those methods -- I would have to think more about this to come up with relevant class(es).

You should document your function, particularly the output.

I really don't see what you can do with the output since it's completely different from one return statement to the next. Maybe you could concatenate all scoring cards in one array and always return that after the score itself. If you do need to return your complicated values, maybe you could define some class for that. Also is_acestraight returns 3 and I'm not sure if that's a mistake.

You could check for return immediately after is_straight and is_acestraight since you just compute useless values if they are true.

I don't think count is a good name since I would expect that to contain the count of something, but it actually contains arrays of cards (where the index is the count).

Since you only use 4 values in count, maybe you could just have defined the 4 variables singles, doubles, triples and quadruples. You never use singles (count[1]), or count[0], so

np.array([np.where(ranks == count)[0] for count in range(2, 5)])

or as I suggested:

doubles = np.array(np.where(ranks == 2)[0])
triples = np.array(np.where(ranks == 3)[0])
quadruples = np.array(np.where(ranks == 4)[0])
\$\endgroup\$

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.