Skip to content

refactor: improve performance for hand calculations further #192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Apricot-S merged 1 commit into from
Feb 7, 2026
Merged

refactor: improve performance for hand calculations further #192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ The following methods are now available as static methods:
- Fixed an issue where `KokushiMusou.is_condition_met()` would return `None` if the condition was not met. It now consistently returns a `bool` value. Remove any `None` checks in the code that relied on the previous behavior.
- `Shanten.calculate_shanten()` and `Shanten.calculate_shanten_for_regular_hand()` now raises `ValueError` instead of `assert` when the number of tiles is 15 or more.
- `HandDivider.divide_hand()` now determines block type from `Meld.type` instead of inferring it from `Meld.tiles`. Behavior may differ for invalid `Meld.tiles` or inconsistent `Meld.type` and `Meld.tiles` combinations.
- Removed `HandCalculator.ERR_HAND_NOT_CORRECT`. Hands that previously returned `ERR_HAND_NOT_CORRECT` now return `ERR_HAND_NOT_WINNING` instead.

## What's Changed
- Placeholder. It would be filled on release automatically
Expand Down
8 changes: 1 addition & 7 deletions mahjong/hand_calculating/hand.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from collections.abc import Collection

from mahjong.agari import Agari
from mahjong.constants import AKA_DORA_LIST, CHUN, HAKU, HATSU
from mahjong.hand_calculating.divider import HandDivider
from mahjong.hand_calculating.fu import FuCalculator
Expand All @@ -23,7 +22,6 @@ class HandCalculator:
ERR_OPEN_HAND_DABURI = "open_hand_daburi_not_allowed"
ERR_IPPATSU_WITHOUT_RIICHI = "ippatsu_without_riichi_not_allowed"
ERR_HAND_NOT_WINNING = "hand_not_winning"
ERR_HAND_NOT_CORRECT = "hand_not_correct"
ERR_NO_YAKU = "no_yaku"
ERR_CHANKAN_WITH_TSUMO = "chankan_with_tsumo_not_allowed"
ERR_RINSHAN_WITHOUT_TSUMO = "rinshan_without_tsumo_not_allowed"
Expand Down Expand Up @@ -82,7 +80,6 @@ def estimate_hand_value(
is_aotenjou = isinstance(scores_calculator, Aotenjou)

opened_melds = [x.tiles_34 for x in melds if x.opened]
all_melds = [x.tiles_34 for x in melds]
is_open_hand = len(opened_melds) > 0

# special situation
Expand Down Expand Up @@ -153,9 +150,6 @@ def estimate_hand_value(
if config.is_renhou and melds:
return HandResponse(error=HandCalculator.ERR_RENHOU_WITH_MELD)

if not Agari.is_agari(tiles_34, all_melds):
return HandResponse(error=HandCalculator.ERR_HAND_NOT_WINNING)

if not config.options.has_double_yakuman:
config.yaku.daburu_kokushi.han_closed = 13
config.yaku.suuankou_tanki.han_closed = 13
Expand Down Expand Up @@ -533,7 +527,7 @@ def estimate_hand_value(
)

if not calculated_hands:
return HandResponse(error=HandCalculator.ERR_HAND_NOT_CORRECT)
return HandResponse(error=HandCalculator.ERR_HAND_NOT_WINNING)
Copy link
Collaborator

@Apricot-S Apricot-S Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No breaking changes are required if we do the following:

hand_options = HandDivider.divide_hand(tiles_34, melds)
is_kokushi = not is_open_hand and config.yaku.kokushi.is_condition_met(None, tiles_34)

if not hand_options and not is_kokushi:
    return HandResponse(error=HandCalculator.ERR_HAND_NOT_WINNING)

...

if not calculated_hands:
    return HandResponse(error=HandCalculator.ERR_HAND_NOT_CORRECT)

Copy link
Contributor Author

@Nihisil Nihisil Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it is worth to keep comparability here? It was really rare case and hack with additional error response. Now we have a chance to fix it properly

Copy link
Collaborator

@Apricot-S Apricot-S Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood.


# find most expensive hand
calculated_hands = sorted(calculated_hands, key=lambda x: (x["han"], x["fu"]), reverse=True)
Expand Down
50 changes: 25 additions & 25 deletions tests/hand_calculating/tests_hand_response_error.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def test_no_winning_tile() -> None:
win_tile = _string_to_136_tile(sou="9")

result = hand.estimate_hand_value(tiles, win_tile, config=_make_hand_config(is_riichi=True))
assert result.error == "winning_tile_not_in_hand"
assert result.error == HandCalculator.ERR_NO_WINNING_TILE


def test_open_hand_riichi() -> None:
Expand All @@ -26,7 +26,7 @@ def test_open_hand_riichi() -> None:

melds = [_make_meld(Meld.CHI, sou="123")]
result = hand.estimate_hand_value(tiles, win_tile, melds=melds, config=_make_hand_config(is_riichi=True))
assert result.error == "open_hand_riichi_not_allowed"
assert result.error == HandCalculator.ERR_OPEN_HAND_RIICHI


def test_open_hand_daburi() -> None:
Expand All @@ -42,7 +42,7 @@ def test_open_hand_daburi() -> None:
melds=melds,
config=_make_hand_config(is_riichi=True, is_daburu_riichi=True),
)
assert result.error == "open_hand_daburi_not_allowed"
assert result.error == HandCalculator.ERR_OPEN_HAND_DABURI


def test_ippatsu_without_riichi() -> None:
Expand All @@ -52,7 +52,7 @@ def test_ippatsu_without_riichi() -> None:
win_tile = _string_to_136_tile(sou="4")

result = hand.estimate_hand_value(tiles, win_tile, config=_make_hand_config(is_ippatsu=True))
assert result.error == "ippatsu_without_riichi_not_allowed"
assert result.error == HandCalculator.ERR_IPPATSU_WITHOUT_RIICHI


def test_hand_not_winning() -> None:
Expand All @@ -62,7 +62,7 @@ def test_hand_not_winning() -> None:
win_tile = _string_to_136_tile(sou="4")

result = hand.estimate_hand_value(tiles, win_tile)
assert result.error == "hand_not_winning"
assert result.error == HandCalculator.ERR_HAND_NOT_WINNING


def test_no_yaku() -> None:
Expand All @@ -73,7 +73,7 @@ def test_no_yaku() -> None:

melds = [_make_meld(Meld.CHI, sou="123")]
result = hand.estimate_hand_value(tiles, win_tile, melds=melds)
assert result.error == "no_yaku"
assert result.error == HandCalculator.ERR_NO_YAKU


def test_chankan_with_tsumo() -> None:
Expand All @@ -83,7 +83,7 @@ def test_chankan_with_tsumo() -> None:
win_tile = _string_to_136_tile(sou="1")

result = hand.estimate_hand_value(tiles, win_tile, config=_make_hand_config(is_tsumo=True, is_chankan=True))
assert result.error == "chankan_with_tsumo_not_allowed"
assert result.error == HandCalculator.ERR_CHANKAN_WITH_TSUMO


def test_rinshan_without_tsumo() -> None:
Expand All @@ -93,7 +93,7 @@ def test_rinshan_without_tsumo() -> None:
win_tile = _string_to_136_tile(sou="4")

result = hand.estimate_hand_value(tiles, win_tile, config=_make_hand_config(is_tsumo=False, is_rinshan=True))
assert result.error == "rinshan_without_tsumo_not_allowed"
assert result.error == HandCalculator.ERR_RINSHAN_WITHOUT_TSUMO


def test_haitei_without_tsumo() -> None:
Expand All @@ -103,7 +103,7 @@ def test_haitei_without_tsumo() -> None:
win_tile = _string_to_136_tile(sou="4")

result = hand.estimate_hand_value(tiles, win_tile, config=_make_hand_config(is_tsumo=False, is_haitei=True))
assert result.error == "haitei_without_tsumo_not_allowed"
assert result.error == HandCalculator.ERR_HAITEI_WITHOUT_TSUMO


def test_houtei_with_tsumo() -> None:
Expand All @@ -113,7 +113,7 @@ def test_houtei_with_tsumo() -> None:
win_tile = _string_to_136_tile(sou="4")

result = hand.estimate_hand_value(tiles, win_tile, config=_make_hand_config(is_tsumo=True, is_houtei=True))
assert result.error == "houtei_with_tsumo_not_allowed"
assert result.error == HandCalculator.ERR_HOUTEI_WITH_TSUMO


def test_haitei_with_rinshan() -> None:
Expand All @@ -127,7 +127,7 @@ def test_haitei_with_rinshan() -> None:
win_tile,
config=_make_hand_config(is_tsumo=True, is_rinshan=True, is_haitei=True),
)
assert result.error == "haitei_with_rinshan_not_allowed"
assert result.error == HandCalculator.ERR_HAITEI_WITH_RINSHAN


def test_houtei_with_chankan() -> None:
Expand All @@ -141,7 +141,7 @@ def test_houtei_with_chankan() -> None:
win_tile,
config=_make_hand_config(is_tsumo=False, is_chankan=True, is_houtei=True),
)
assert result.error == "houtei_with_chankan_not_allowed"
assert result.error == HandCalculator.ERR_HOUTEI_WITH_CHANKAN


def test_tenhou_not_as_dealer() -> None:
Expand All @@ -160,7 +160,7 @@ def test_tenhou_not_as_dealer() -> None:
win_tile,
config=_make_hand_config(is_tsumo=True, is_tenhou=True, player_wind=SOUTH),
)
assert result.error == "tenhou_not_as_dealer_not_allowed"
assert result.error == HandCalculator.ERR_TENHOU_NOT_AS_DEALER


def test_tenhou_without_tsumo() -> None:
Expand All @@ -170,7 +170,7 @@ def test_tenhou_without_tsumo() -> None:
win_tile = _string_to_136_tile(sou="4")

result = hand.estimate_hand_value(tiles, win_tile, config=_make_hand_config(is_tsumo=False, is_tenhou=True))
assert result.error == "tenhou_without_tsumo_not_allowed"
assert result.error == HandCalculator.ERR_TENHOU_WITHOUT_TSUMO


def test_tenhou_with_meld() -> None:
Expand All @@ -186,7 +186,7 @@ def test_tenhou_with_meld() -> None:
melds=melds,
config=_make_hand_config(is_tsumo=True, is_rinshan=True, is_tenhou=True),
)
assert result.error == "tenhou_with_meld_not_allowed"
assert result.error == HandCalculator.ERR_TENHOU_WITH_MELD


def test_chiihou_as_dealer() -> None:
Expand All @@ -205,7 +205,7 @@ def test_chiihou_as_dealer() -> None:
win_tile,
config=_make_hand_config(is_tsumo=True, is_chiihou=True, player_wind=EAST),
)
assert result.error == "chiihou_as_dealer_not_allowed"
assert result.error == HandCalculator.ERR_CHIIHOU_AS_DEALER


def test_chiihou_without_tsumo() -> None:
Expand All @@ -215,7 +215,7 @@ def test_chiihou_without_tsumo() -> None:
win_tile = _string_to_136_tile(sou="4")

result = hand.estimate_hand_value(tiles, win_tile, config=_make_hand_config(is_tsumo=False, is_chiihou=True))
assert result.error == "chiihou_without_tsumo_not_allowed"
assert result.error == HandCalculator.ERR_CHIIHOU_WITHOUT_TSUMO


def test_chiihou_with_meld() -> None:
Expand All @@ -231,7 +231,7 @@ def test_chiihou_with_meld() -> None:
melds=melds,
config=_make_hand_config(is_tsumo=True, is_rinshan=True, is_chiihou=True),
)
assert result.error == "chiihou_with_meld_not_allowed"
assert result.error == HandCalculator.ERR_CHIIHOU_WITH_MELD


def test_renhou_as_dealer() -> None:
Expand All @@ -250,7 +250,7 @@ def test_renhou_as_dealer() -> None:
win_tile,
config=_make_hand_config(is_tsumo=False, is_renhou=True, player_wind=EAST),
)
assert result.error == "renhou_as_dealer_not_allowed"
assert result.error == HandCalculator.ERR_RENHOU_AS_DEALER


def test_renhou_with_tsumo() -> None:
Expand All @@ -260,7 +260,7 @@ def test_renhou_with_tsumo() -> None:
win_tile = _string_to_136_tile(sou="4")

result = hand.estimate_hand_value(tiles, win_tile, config=_make_hand_config(is_tsumo=True, is_renhou=True))
assert result.error == "renhou_with_tsumo_not_allowed"
assert result.error == HandCalculator.ERR_RENHOU_WITH_TSUMO


def test_renhou_with_meld() -> None:
Expand All @@ -276,7 +276,7 @@ def test_renhou_with_meld() -> None:
melds=melds,
config=_make_hand_config(is_tsumo=False, is_renhou=True),
)
assert result.error == "renhou_with_meld_not_allowed"
assert result.error == HandCalculator.ERR_RENHOU_WITH_MELD


def test_win_tile_only_in_opened_meld() -> None:
Expand All @@ -291,15 +291,15 @@ def test_win_tile_only_in_opened_meld() -> None:

meld = Meld(meld_type=Meld.PON, tiles=tiles[0:3], opened=True, called_tile=tiles[0], who=0)
result = hand.estimate_hand_value(tiles, win_tile, melds=[meld])
assert result.error == "hand_not_correct"
assert result.error == HandCalculator.ERR_HAND_NOT_WINNING


@pytest.mark.parametrize(
"error_string",
[
"hand_not_winning",
"no_yaku",
"winning_tile_not_in_hand",
HandCalculator.ERR_HAND_NOT_WINNING,
HandCalculator.ERR_NO_YAKU,
HandCalculator.ERR_NO_WINNING_TILE,
],
)
def test_str_returns_error_when_error_is_set(error_string: str) -> None:
Expand Down
44 changes: 35 additions & 9 deletions tests/tests_agari.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,17 @@ def test_honor_tile_overflow_returns_false(tiles_34: list[int]) -> None:
assert Agari.is_agari(tiles_34) is False


def test_pin_suit_mod3_equals_1_returns_false() -> None:
# 1 tile in pin suit: sum = 1, 1 % 3 = 1
tiles = TilesConverter.string_to_34_array(pin="1")
assert Agari.is_agari(tiles) is False


def test_sou_suit_mod3_equals_1_returns_false() -> None:
# 1 tile in sou suit: sum = 1, 1 % 3 = 1
tiles = TilesConverter.string_to_34_array(sou="1")
@pytest.mark.parametrize(
("man", "pin", "sou"),
[
pytest.param("1", "", "", id="man"),
pytest.param("", "1", "", id="pin"),
pytest.param("", "", "1", id="sou"),
],
)
def test_single_tile_in_suit_returns_false(man: str, pin: str, sou: str) -> None:
# a single tile in any suit can't form a valid group (tile count % 3 == 1)
tiles = TilesConverter.string_to_34_array(man=man, pin=pin, sou=sou)
assert Agari.is_agari(tiles) is False


Expand Down Expand Up @@ -148,3 +150,27 @@ def test_is_mentsu_negative_a_returns_false() -> None:
# 1m count=2 implies 2 sequences, but 2m count=0 gives a = 0 - 2 = -2 < 0
tiles = TilesConverter.string_to_34_array(man="114", honors="11")
assert Agari.is_agari(tiles) is False


def test_open_hand_with_kan_meld() -> None:
# 4-tile kan meld exercises the len(meld) > 3 branch
tiles = TilesConverter.string_to_34_array(man="1111", pin="123456789", sou="22")
kan_of_1m = [0, 0, 0, 0]
assert Agari.is_agari(tiles, [kan_of_1m]) is True


@pytest.mark.parametrize(
("man", "pin"),
[
pytest.param("66", "111222333444"),
pytest.param("99", "111222333444"),
pytest.param("22", "111222333444"),
pytest.param("44", "111222333444"),
pytest.param("77", "111222333444"),
],
)
def test_atama_mentsu_pair_positions(man: str, pin: str) -> None:
# each case places the pair at a different position within the man suit,
# exercising distinct branches in _is_atama_mentsu
tiles = TilesConverter.string_to_34_array(man=man, pin=pin)
assert Agari.is_agari(tiles) is True