Resolve edge cases in test_dptables.py

[azriel1rf]

In the test_dptables.py file, there is a TODO comment that needs to be addressed.
The comment is located in the loop that generates combinations and updates the table variable, specifically in the case where idx is in [72, 520, 576].

Here's the relevant code:

if idx in [72, 520, 576] and SUITS[idx] != suit + 1:
    continue

The TODO comment suggests that there might be an issue with the way the table is being updated when idx is one of the specified values. This needs to be investigated and resolved.

Acceptance Criteria:

• Implement a fix if necessary.
• Add or update unit tests to cover these specific cases.
• Ensure that all tests pass after the changes.
• Remove the TODO comment once the issue has been resolved.

[HenryRLee]

I figured out what this issue is. This test case actually showed a bug using the SUITS table in 9-card evaluation. The original SUITS table uses 12-bit binaries as the key. So that each suit has 3 bits. That means each suit can hold at most 8 different states (0 to 7 cards). In the 9-card case, when one suit has 8 cards and another suit has 1 card, the suit with 8 cards will have an overflow.

For example, as the code mentioned, number 520, 0b1000001000. This could represent the lowest suit has 8 cards, and the highest suit has 1 card. But also, it may represent the second lowest suit has 1 card, and the second highest suit has 8 cards. So here is the conflict.

In order to fix this bug, in the later implementation for 9-card evaluation, I discarded using the SUITS table, and just counted the suits one by one. Later on, the 9-card evaluation code had been deprecated, and had been replaced by Omaha evaluation, which still doesn't use the SUITS table.

Therefore, to fix this test case, I think we can just avoid testing the 9-card case (and 8-card case can be dropped too). And the whole block under the TODO comment can be removed safely.

[azriel1rf]

Thank you for explaining the bug in the SUITS table for 9-card evaluations in the C++ implementation.
However, the Python implementation in evaluator.py still uses the SUITS table:

def _evaluate_cards(*cards: int) -> int:
    ...
    flush_suit = SUITS[suit_hash] - 1

PokerHandEvaluator/python/phevaluator/evaluator.py

Line 65 in 1bc79a2

flush_suit = SUITS[suit_hash] - 1

Simply removing the 8-card and 9-card test cases in test_dptables.py without updating SUITS or modifying the Python code may lead to inconsistencies.

I suggest:

• If SUITS is no longer needed in Python, update evaluator.py to remove the dependency and implement suit counting like in C++.
• If SUITS is still required, update it in dptables.py to handle them up to 7-card correctly.
  • Then, update test_dptables.py with test cases for the updated SUITS or modified evaluator.py.

Let me know if you have any further questions.

[HenryRLee]

But we don't use _evaluate_cards for 8 and 9 cards anymore. These have been deprecated.

    if not (MIN_CARDS <= hand_size <= MAX_CARDS) or (hand_size not in NO_FLUSHES):
        msg = (
            f"The number of cards must be between {MIN_CARDS} and {MAX_CARDS}."
            f"passed size: {hand_size}"
        )
        raise ValueError(msg)

    return _evaluate_cards(*int_cards)

Here MAX_CARDS is 7.

[HenryRLee]

Ah, I get your point. I am currently working on this suggestion.

If SUITS is still required, update it in dptables.py to handle them up to 7-card correctly.
Then, update test_dptables.py with test cases for the updated SUITS or modified evaluator.py.