Fix nit in noisy_policy

If player_id is 0 then not player_id is True, however we only want not player_id to be True in the case of player_id = None. Therefore, I have changed this to test player_id is None. I have updated the tests. I guess the test in the test is due to stochasticity of the output of the test (b/141737795). I wonder if noisy_policy has been ever used, it seems that even this initialization is not correct: https://github.com/deepmind/open_spiel/blob/38941dee3beb52ffdb134b66f420a758634d9a20/open_spiel/python/algorithms/best_response.py#L318
google-deepmind · Sep 8, 2021 · 2703b20 · 2703b20
1 parent 38941de
commit 2703b20
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 8 deletions.
diff --git a/open_spiel/python/algorithms/noisy_policy.py b/open_spiel/python/algorithms/noisy_policy.py
@@ -113,11 +113,11 @@ def action_probabilities(self, state, player_id=None):
 
     # If self._player_id is None, or if self.player_id == current_player, add
     # noise.
-    if (not self.player_id) or (state.current_player() == self.player_id):
+    if (self.player_id is None) or (state.current_player() == self.player_id):
       noise_probs = self.get_or_create_noise(state)
       probs = self._policy.action_probabilities(state, player_id)
       probs = self.mix_probs(probs, noise_probs)
       return probs
 
     # Send the default probabilities for all other players
-    return self._policy.action_probabilities(state, player_id)
+    return self._policy.action_probabilities(state)
diff --git a/open_spiel/python/algorithms/noisy_policy_test.py b/open_spiel/python/algorithms/noisy_policy_test.py
@@ -40,15 +40,21 @@ def test_cpp_and_python_implementations_are_identical(self, game_name):
         to_string=lambda s: s.information_state_string())
 
     for current_player in range(game.num_players()):
-      noise = noisy_policy.NoisyPolicy(policy, 0, alpha=0.5, beta=10.)
+      noise = noisy_policy.NoisyPolicy(
+        policy, player_id=current_player, alpha=0.5, beta=10.)
       for state in all_states.values():
-        if state.current_player() != current_player:
+        if state.current_player() < 0:
           continue
 
-        # TODO(b/141737795): Decide what to do about this.
-        self.assertNotEqual(
-            policy.action_probabilities(state),
-            noise.action_probabilities(state))
+        if state.current_player() != current_player:
+          self.assertEqual(
+              policy.action_probabilities(state),
+              noise.action_probabilities(state))
+        else:
+          # TODO(b/141737795): Decide what to do about this.
+          self.assertNotEqual(
+              policy.action_probabilities(state),
+              noise.action_probabilities(state))
 
 if __name__ == "__main__":
   absltest.main()