Simplifies TabularBestResponse API & adds test verifying correctness.

PiperOrigin-RevId: 266414964
NgomGit · Sep 3, 2019 · 3c454fb · 3c454fb
1 parent 09ebf37
commit 3c454fb
Show file tree

Hide file tree

Showing 43 changed files with 110 additions and 358 deletions.
diff --git a/README.md b/README.md
@@ -1,9 +1,3 @@
-<!--- BEGIN GOOGLE-INTERNAL
-
-This file is the main page of the repo, and contains sub-parts of the
-documentation. Update `docs/intro.md` too if you update this page.
-
-END GOOGLE-INTERNAL -->
 
 # OpenSpiel: A Framework for Reinforcement Learning in Games
 
@@ -39,139 +33,3 @@ Please choose among the following options:
 *   [Swift OpenSpiel](docs/swift.md)
 *   [Authors](docs/authors.md)
 
-## BEGIN GOOGLE-INTERNAL
-
-Player of Games is also using the API. The code is available in:
-cs/google3/learning/deepmind/research/mcts/impinfo/pog/engine/open_spiel_state.h
-
-TODO(jblespiau): In the OpenSource version, should `scripts/` be at the top
-level directory?
-
-## Git-on-Borg (GoB) and release process
-
-We now have the GoB team review site at
-[OpenSpiel Team Review Repository](https://team-review.git.corp.google.com/admin/repos/deepmind-eng/OpenSpiel).
-
-You can browse the source at the source
-[GoB source browser](https://team.git.corp.google.com/deepmind-eng/OpenSpiel)
-
-NOTE: The current approach is more toward considering Piper as the Source of
-Truth. Given that our code is currently only in Piper, it is mandatory to create
-the process to go smoothly from Piper to GitHub. Note that setting the process
-in one way allows to do the import the other-way around quite easily. So
-importing Pull Request from GitHub will be supported, but will be added later.
-
-To configure Git-on-Borg, we used the following resources:
-
--   http://go/dm-opensource#StageGoB
--   go/gob/users/team-repository (delete?)
--   go/copybara-piper-sot and
-    https://g3doc.corp.google.com/devtools/copybara/docs/userdoc/codelabs/piper_to_git.md
-
-Please setup the alias `alias
-copybara='/google/data/ro/teams/copybara/copybara'` in your `~/.bashrc`.
-
-### Performing a push and visualizing the result
-
-Before pushing to Git-on-Borg, please first:
-
--   `g4 sync`
--   Run`scripts/google_run_tests.sh`
--   Run`scripts/google_run_swift_tests.sh`
-
-For the initial push:
-
--   `copybara copy.bara.sky piper_to_gob --init-history --squash --force` for
-    the initial push. (And note the latest CL number!)
--   `git clone "sso://team/deepmind-eng/OpenSpiel"` and look at the result.
-
-For the second push:
-
--   This step might be unnecessary. Try first without --last-rev.
--   `copybara copy.bara.sky piper_to_gob --last-rev <CL number from above>`.
-    Note: It is possible that the --last-rev is only needed because we did not
-    have the piper_to_gob transform set with mode="ITERATIVE"
-
-For all subsequent pushes:
-
--   Run `copybara copy.bara.sky piper_to_gob`.
-
-If you get an error claiming that the commit message is empty (like this:
-https://paste.googleplex.com/5354562577760256) then, for every CL that causes
-this, it was necessary to force the commit message using:
-
-`copybara copy.bara.sky piper_to_gob --force-message "BEGIN_PUBLIC\nInternal
-change\nEND_PUBLIC\n" --iterative-limit-changes 1`
-
-that I found here:
-https://groups.google.com/a/google.com/forum/#!msg/copybara-users/E-XdD0KsAQs/rIMyEh_QAwAJ
-
-### Pushing to Github
-
-First, you will need to have write permissions on
-https://github.com/deepmind/open_spiel and have added an
-[SSH private key](https://help.github.com/en/articles/generating-a-new-ssh-key-and-adding-it-to-the-ssh-agent)
-to be able to push to this directory (note that you will need to add the your
-ssh-agent every time you create a new shell).
-
-**Preferred solution**
-
-Push to GoB and then
-
-```
-git clone "sso://team/deepmind-eng/OpenSpiel"
-cd OpenSpiel
-# You can look at the commit history using e.g. `gitk`
-git remote add github git@github.com:deepmind/open_spiel.git
-# See https://help.github.com/en/articles/generating-a-new-ssh-key-and-adding-it-to-the-ssh-agent
-# for how to setup an SSH key to identify on Github.
-eval "$(ssh-agent -s)"
-ssh-add ~/.ssh/github
-git push github master
-```
-
-NOTE: For the first push, you will need to use `--force` to override the
-history.
-
-#### Readthedoc
-
-Website: https://readthedocs.org/ \
-Emails in the account: open_spiel@google.com
-(primary),lanctot@google.com,jblespiau@google.com \
-Username: open_spiel \
-Password: Ask jblespiau@, lanctot@, locked@ or vzambaldi@
-
-#### Travis CI
-
-Website: https://travis-ci.org/
-
-### Historical commands to setup CopyBara/Git-on-Borg
-
-This is to document what has been done. Please update it if you perform other
-modifications to the GoB configuration/adjust permissions, etc.
-
--   Added mdformatting support for OpenSpiel in
-    cs/devtools/markdown/mdformat/depot_path.cc (cr/248682891)
--   Create a GoB repository using the
-    [UI](https://team-review.git.corp.google.com/x/createproject/create), owned
-    by `deepmind-eng`
--   [Whitelist](https://g3doc.corp.google.com/devtools/copybara/docs/userdoc/piper_to_git.md?cl=head#before-you-start-whitelisting-your-piper-paths)
-    the Piper path for CopyBara (see cr/248707174)
--   Executed the first export (as in
-    [this documentation](http://go/copybara-piper-sot#first_run)): `copybara
-    copy.bara.sky piper_to_gob --init-history --squash --force`
-
-TODO(jblespiau):
-
--   Add github account
-    (http://go/dm-opensource#4-create-the-external-repository).
--   Setup Git submodules.
--   Change the GoB setup to use the
-    [`ITERATIVE`](http://go/copybara-git-sot#importing-all-the-history-iterative-mode).
-    mode, and document how to write commit messages. See
-    cs/google3/third_party/py/sonnet/METADATA?type=cs&q=sonnet+BEGIN_PUBLIC&g=0&l=53&rcl=247583839
--   Decide about how we want to expose ldaps in the history.
--   Setup https://readthedocs.org.
--   Setup Travis CI.
-
-END GOOGLE-INTERNAL
diff --git a/install.sh b/install.sh
@@ -14,45 +14,6 @@
 
 #!/usr/bin/env bash
 
-# BEGIN GOOGLE-INTERNAL
-# This comment aims at documenting the decision process.
-#
-# For OpenSourcing information in Google, see:
-# go/big-opensource-project, go/rmi-opensource (for one shot release) and
-# go/dm-opensource
-# Given we aim for a long term project, our project is probably to be considered
-# as a large one, not a one shot release.
-#
-#
-# Possible options for building:
-# - bazel
-# - cmake
-# bazel may be easier to maintain in the long run but it seems it may not
-# support all features we need (e.g. cross langage dependencies). cmake is
-# already setup and seems easy to update, so keeping it seems a good choice for
-# now, but we may want to revisit it.
-#
-# Possible options for testing:
-# - internally with Kokoro (e.g. done in Sonnet)
-# - on Github with Travis
-# The later is probably simpler at first, even though the second can detect
-# breakage *before* releasing to Github. Given our manpower, let's go for easy.
-# Note that Kokoro should be reasonable to setup (1/2 to 1 day).
-#
-# Dependencies: We want users to be able to have a one line install.
-# Installing the dependencies (e.g. C++ Abseil) for the full system can be
-# harder to setup and there may be version-conflicts. Thus we will have our
-# specific depencencies only for OpenSpiel.
-# The structure of the repo is currently:
-#  ./: This is the root of the git repo.
-#  open_spiel/: The code for OpenSpiel
-#  pybind11/: The for for a specific dependency (here pybind11).
-#
-# TODO(jblespiau): For double_dummy_solver, we do not put it at the root, but
-# within `open_spiel/games/bridge/double_dummy_solver`. Reassess that.
-#
-# To see the structure, run ./google_install_and_run_tests.sh nobuild
-# END GOOGLE-INTERNAL
 
 # The following should be easy to setup as a submodule:
 # https://git-scm.com/docs/git-submodule
@@ -78,7 +39,7 @@ else
 fi
 
 git clone -b 'v2.2.4' --single-branch --depth 1 https://github.com/pybind/pybind11.git
-# TODO(jblespiau): Point to the official  https://github.com/dds-bridge/dds.git
+# TODO: Point to the official  https://github.com/dds-bridge/dds.git
 # when pull requests are in
 git clone -b 'develop' --single-branch --depth 1 https://github.com/jblespiau/dds.git  open_spiel/games/bridge/double_dummy_solver
 git clone -b 'master' --single-branch --depth 1 https://github.com/abseil/abseil-cpp.git open_spiel/abseil-cpp
diff --git a/open_spiel/algorithms/history_tree.cc b/open_spiel/algorithms/history_tree.cc
@@ -185,7 +185,7 @@ ActionsAndProbs GetSuccessorsWithProbs(const State& state,
   }
 }
 
-// TODO(finbarrtimbers): If this is a bottleneck, it should be possible
+// TODO: If this is a bottleneck, it should be possible
 // to pass the probabilities-so-far into the call, and get everything right
 // the first time, without recursion. The recursion is simpler, however.
 std::vector<std::pair<std::unique_ptr<State>, double>> DecisionNodes(

diff --git a/open_spiel/algorithms/history_tree.h b/open_spiel/algorithms/history_tree.h
@@ -27,7 +27,7 @@
 namespace open_spiel {
 namespace algorithms {
 
-// TODO(finbarrtimbers): See if it's possible to remove any fields here.
+// TODO: See if it's possible to remove any fields here.
 // Stores all information relevant to exploitability calculation for each
 // history in the game.
 class HistoryNode {

diff --git a/open_spiel/algorithms/history_tree_test.cc b/open_spiel/algorithms/history_tree_test.cc
@@ -48,7 +48,7 @@ void TestGameTree() {
     for (int player_id : {0, 1}) {
       HistoryTree tree(game->NewInitialState(), player_id);
       if (tree.NumHistories() != num_histories[game_name]) {
-        // TODO(b/126764761): Replace calls to SpielFatalError with more
+        // TODO: Replace calls to SpielFatalError with more
         // appropriate test macros once they support logging.
         SpielFatalError(absl::StrCat(
             "In the game ", game_name,

diff --git a/open_spiel/algorithms/tabular_exploitability.cc b/open_spiel/algorithms/tabular_exploitability.cc
@@ -92,7 +92,7 @@ double TabularBestResponse::HandleDecisionCase(HistoryNode* node) {
       std::vector<std::string> action_probs_str_vector;
       action_probs_str_vector.reserve(state_policy.size());
       for (const auto& action_prob : state_policy) {
-        // TODO(b/b/127423396): Use absl::StrFormat.
+        // TODO: Use absl::StrFormat.
         action_probs_str_vector.push_back(absl::StrCat(
             "(", action_prob.first, ", ", action_prob.second, ")"));
       }

diff --git a/open_spiel/algorithms/tabular_exploitability.h b/open_spiel/algorithms/tabular_exploitability.h
@@ -74,12 +74,14 @@ class TabularBestResponse {
   Action BestResponseAction(const std::string& infostate);
 
   // Returns a map of infostates to best responses, for all information states
-  // that have been calculated so far.
-  // If Value or best response has been calculated for the root of the game,
-  // this will be every information state in the game.
+  // that have been calculated so far. If no best responses have been
+  // calculated, then we calculate them for every state in the game.
   // When two actions have the same value, we
   // return the action with the lowest number (as an int).
   std::unordered_map<std::string, Action> GetBestResponseActions() {
+    // If the best_response_actions_ cache is empty, we fill it by calculating
+    // all best responses, starting at the root.
+    if (best_response_actions_.empty()) Value(root_->ToString());
     return best_response_actions_;
   }
 
@@ -94,7 +96,7 @@ class TabularBestResponse {
     policy_ = policy;
     value_cache_.clear();
     best_response_actions_.clear();
-    // TODO(finbarrtimbers): Replace this with something that traverses the tree
+    // TODO: Replace this with something that traverses the tree
     // and rebuilds the probabilities.
     infosets_ =
         GetAllInfoSets(root_->Clone(), best_responder_id_, policy_, &tree_);