diff --git a/changes/176.bugfix.rst b/changes/176.bugfix.rst new file mode 100644 index 0000000..ab6d6e4 --- /dev/null +++ b/changes/176.bugfix.rst @@ -0,0 +1 @@ +Fix PlackettLuce rating adjustment for ties. diff --git a/openskill/models/weng_lin/plackett_luce.py b/openskill/models/weng_lin/plackett_luce.py index 44c8e4d..aba3d1d 100644 --- a/openskill/models/weng_lin/plackett_luce.py +++ b/openskill/models/weng_lin/plackett_luce.py @@ -848,7 +848,7 @@ def _compute( delta += ( i_mu_over_ce_over_sum_q * (1 - i_mu_over_ce_over_sum_q) / a[q] ) - if team_q.rank == team_i.rank: + if q == i: omega += (1 - i_mu_over_ce_over_sum_q) / a[q] else: omega -= i_mu_over_ce_over_sum_q / a[q] diff --git a/tests/models/data/plackettluce.json b/tests/models/data/plackettluce.json index a9a0241..36937a7 100644 --- a/tests/models/data/plackettluce.json +++ b/tests/models/data/plackettluce.json @@ -86,21 +86,21 @@ ], "team_2": [ { - "mu": 23.083700519022745, + "mu": 20.599656070925114, "sigma": 8.222674397835641 }, { - "mu": 21.717476072569045, + "mu": 20.47545384852023, "sigma": 8.277466291367071 } ], "team_3": [ { - "mu": 21.717476072569045, + "mu": 20.47545384852023, "sigma": 8.277466291367071 }, { - "mu": 23.083700519022745, + "mu": 20.599656070925114, "sigma": 8.222674397835641 } ], @@ -160,7 +160,7 @@ "ties": { "team_1": [ { - "mu": 23.241111866333558, + "mu": 21.63766806988004, "sigma": 8.310709773172306 } ], @@ -176,15 +176,15 @@ ], "team_3": [ { - "mu": 21.479966996534408, + "mu": 19.87652320008089, "sigma": 8.237522411103104 }, { - "mu": 21.479966996534408, + "mu": 19.87652320008089, "sigma": 8.237522411103104 }, { - "mu": 21.479966996534408, + "mu": 19.87652320008089, "sigma": 8.237522411103104 } ] @@ -261,4 +261,4 @@ } ] } -} \ No newline at end of file +} diff --git a/tests/models/weng_lin/test_common.py b/tests/models/weng_lin/test_common.py index 44e8cd7..f0f8d91 100644 --- a/tests/models/weng_lin/test_common.py +++ b/tests/models/weng_lin/test_common.py @@ -205,20 +205,52 @@ def test_ladder_pairs(): @pytest.mark.parametrize("model", MODELS) @pytest.mark.parametrize("tie_score", [-1, 0, 0.1, 10, 13.4]) -def test_ties(model, tie_score): +@pytest.mark.parametrize("num_teams", [2, 5, 10]) +@pytest.mark.parametrize("team_size", [1, 2, 5, 10]) +@pytest.mark.parametrize("tie_type", ["score", "rank"]) +def test_ties(model, tie_score, num_teams, team_size, tie_type) -> None: model_instance = model() + teams = [ + [model_instance.rating() for _ in range(team_size)] for _ in range(num_teams) + ] + player_mu_before = [player.mu for team in teams for player in team] + assert all( + mu == player_mu_before[0] for mu in player_mu_before + ), f"Model {model.__name__} with score {tie_score}: All players should start with equal mu" + + player_sigma_before = [player.sigma for team in teams for player in team] + assert all( + sigma == player_sigma_before[0] for sigma in player_sigma_before + ), f"Model {model.__name__} with score {tie_score}: All players should start with equal sigma" + + if tie_type == "score": + scores = [tie_score for _ in range(num_teams)] + new_teams = model_instance.rate(teams, scores=scores) + else: # rank + ranks = [tie_score for _ in range(num_teams)] + new_teams = model_instance.rate(teams, ranks=ranks) + + player_mu_after = [player.mu for team in new_teams for player in team] + assert all( + mu_after == mu_before + for mu_after, mu_before in zip(player_mu_after, player_mu_before) + ), f"Model {model.__name__} with score {tie_score}: All players should end with equal mu" + player_sigma_after = [player.sigma for team in new_teams for player in team] + assert all( + sigma_after <= sigma_before + for sigma_after, sigma_before in zip(player_sigma_after, player_sigma_before) + ), f"Model {model.__name__} with score {tie_score}: All players should end with lower or equal sigma" - player_1 = model_instance.rating() - player_2 = model_instance.rating() - result = model_instance.rate( - [[player_1], [player_2]], scores=[tie_score, tie_score] - ) +@pytest.mark.parametrize("model", MODELS) +def test_ties_with_close_ratings(model) -> None: + model_instance = model() + + player_1 = model_instance.rating(mu=30) + player_2 = model_instance.rating(mu=20) + + new_teams = model_instance.rate([[player_1], [player_2]], ranks=[0, 0]) - # Both players should have the same rating change - assert ( - result[0][0].mu == result[1][0].mu - ), f"Model {model.__name__} with score {tie_score}: Players should have equal mu after tie" - assert ( - result[0][0].sigma == result[1][0].sigma - ), f"Model {model.__name__} with score {tie_score}: Players should have equal sigma after tie" + # ratings should converge on ties. + assert new_teams[0][0].mu < 30 + assert new_teams[1][0].mu > 20