Overhaul logging and deprecate the compiler_gym.util.logs and compiler_gym.replay_search modules

Makefile

@@ -275,12 +275,12 @@ livedocs: gendocs doxygen
 # Testing #
 ###########
 
-COMPILER_GYM_SITE_DATA ?= "/tmp/compiler_gym_$(USER)/tests/site_data"
-COMPILER_GYM_CACHE ?= "/tmp/compiler_gym_$(USER)/tests/cache"
+COMPILER_GYM_SITE_DATA ?= /tmp/compiler_gym_$(USER)/tests/site_data
+COMPILER_GYM_CACHE ?= /tmp/compiler_gym_$(USER)/tests/cache
 
 # A directory that is used as the working directory for running pytest tests
 # by symlinking the tests directory into it.
-INSTALL_TEST_ROOT ?= "/tmp/compiler_gym_$(USER)/install_tests"
+INSTALL_TEST_ROOT ?= /tmp/compiler_gym_$(USER)/install_tests
 
 # The target to use. If not provided, all tests will be run. For `make test` and
 # related, this is a bazel target pattern, with default value '//...'. For `make
@@ -307,7 +307,6 @@ itest: bazel-fetch
 install-test-setup:
 	mkdir -p "$(INSTALL_TEST_ROOT)"
 	rm -f "$(INSTALL_TEST_ROOT)/tests" "$(INSTALL_TEST_ROOT)/tox.ini"
-	ln -s "$(INSTALL_TEST_ROOT)"
 	ln -s "$(ROOT)/tests" "$(INSTALL_TEST_ROOT)"
 	ln -s "$(ROOT)/tox.ini" "$(INSTALL_TEST_ROOT)"
 
@@ -360,10 +359,12 @@ install: |  init-runtime-requirements bazel-build pip-install
 # A list of all filesystem locations that CompilerGym may use for storing
 # files and data.
 COMPILER_GYM_DATA_FILE_LOCATIONS = \
-    $(HOME)/.cache/compiler_gym \
-    $(HOME)/.local/share/compiler_gym \
-    $(HOME)/logs/compiler_gym \
+    "$(HOME)/.cache/compiler_gym" \
+    "$(HOME)/.local/share/compiler_gym" \
+    "$(HOME)/logs/compiler_gym" \
     /dev/shm/compiler_gym \
+    /dev/shm/compiler_gym_$(USER) \
+    /tmp/compiler_gym \
     /tmp/compiler_gym_$(USER) \
     $(NULL)
 

compiler_gym/BUILD

@@ -12,6 +12,7 @@ py_library(
     srcs = ["__init__.py"],
     visibility = ["//visibility:public"],
     deps = [
+        ":random_replay",
         ":random_search",
         ":validate",
         "//compiler_gym/bin",
@@ -41,6 +42,7 @@ py_library(
     srcs = ["random_replay.py"],
     visibility = ["//visibility:public"],
     deps = [
+        ":random_search",
         "//compiler_gym/envs",
         "//compiler_gym/util",
     ],
@@ -52,7 +54,6 @@ py_library(
     data = ["//compiler_gym/envs/llvm/service"],
     visibility = ["//visibility:public"],
     deps = [
-        ":random_replay",
         "//compiler_gym/envs",
         "//compiler_gym/service:connection",
         "//compiler_gym/util",

compiler_gym/bin/BUILD

@@ -45,6 +45,7 @@ py_binary(
     name = "random_eval",
     srcs = ["random_eval.py"],
     deps = [
+        "//compiler_gym:random_search",
         "//compiler_gym/util",
         "//compiler_gym/util/flags",
     ],
@@ -65,7 +66,7 @@ py_binary(
     srcs = ["random_replay.py"],
     visibility = ["//visibility:public"],
     deps = [
-        "//compiler_gym:random_replay",
+        "//compiler_gym:random_search",
         "//compiler_gym/util",
         "//compiler_gym/util/flags",
     ],

compiler_gym/bin/random_eval.py

@@ -11,6 +11,7 @@
 from absl import app, flags
 
 import compiler_gym.util.flags.output_dir  # noqa Flag definition.
+from compiler_gym.random_search import RandomSearchProgressLogEntry
 from compiler_gym.util import logs
 from compiler_gym.util.statistics import geometric_mean
 from compiler_gym.util.tabulate import tabulate
@@ -46,7 +47,7 @@ def eval_logs(outdir: Path) -> None:
 
         with open(str(progress_path)) as f:
             final_line = f.readlines()[-1]
-        best = logs.ProgressLogEntry.from_csv(final_line)
+        best = RandomSearchProgressLogEntry.from_csv(final_line)
 
         totals["instructions"] += meta["num_instructions"]
         totals["init_reward"].append(meta["init_reward"])

compiler_gym/bin/random_replay.py

@@ -17,7 +17,7 @@
 from absl import app, flags
 
 import compiler_gym.util.flags.output_dir  # noqa Flag definition.
-from compiler_gym.random_replay import replay_actions_from_logs
+from compiler_gym.random_search import replay_actions_from_logs
 from compiler_gym.util import logs
 from compiler_gym.util.flags.benchmark_from_flags import benchmark_from_flags
 from compiler_gym.util.flags.env_from_flags import env_from_flags

compiler_gym/datasets/dataset.py

@@ -10,11 +10,18 @@
 from typing import Dict, Iterable, Optional, Union
 
 import numpy as np
+from deprecated.sphinx import deprecated
 from deprecated.sphinx import deprecated as mark_deprecated
 
 from compiler_gym.datasets.benchmark import Benchmark
 from compiler_gym.datasets.uri import DATASET_NAME_RE
-from compiler_gym.util.debug_util import get_logging_level
+
+logger = logging.getLogger(__name__)
+
+# NOTE(cummins): This is only required to prevent a name conflict with the now
+# deprecated Dataset.logger attribute. This can be removed once the logger
+# attribute is removed, scheduled for release 0.2.3.
+_logger = logger
 
 
 class Dataset:
@@ -43,7 +50,6 @@ def __init__(
         references: Optional[Dict[str, str]] = None,
         deprecated: Optional[str] = None,
         sort_order: int = 0,
-        logger: Optional[logging.Logger] = None,
         validatable: str = "No",
     ):
         """Constructor.
@@ -100,7 +106,6 @@ def __init__(
         self._deprecation_message = deprecated
         self._validatable = validatable
 
-        self._logger = logger
         self.sort_order = sort_order
         self.benchmark_class = benchmark_class
 
@@ -112,19 +117,19 @@ def __repr__(self):
         return self.name
 
     @property
+    @deprecated(
+        version="0.2.1",
+        reason=(
+            "The `Dataset.logger` attribute is deprecated. All Dataset "
+            "instances share a logger named compiler_gym.datasets"
+        ),
+    )
     def logger(self) -> logging.Logger:
         """The logger for this dataset.
 
         :type: logging.Logger
         """
-        # NOTE(cummins): Default logger instantiation is deferred since in
-        # Python 3.6 the Logger instance contains an un-pickle-able Rlock()
-        # which can prevent gym.make() from working. This is a workaround that
-        # can be removed once Python 3.6 support is dropped.
-        if self._logger is None:
-            self._logger = logging.getLogger("compiler_gym.datasets")
-            self._logger.setLevel(get_logging_level())
-        return self._logger
+        return _logger
 
     @property
     def name(self) -> str:

compiler_gym/datasets/tar_dataset.py

@@ -5,6 +5,7 @@
 import bz2
 import gzip
 import io
+import logging
 import shutil
 import tarfile
 from threading import Lock
@@ -17,6 +18,8 @@
 from compiler_gym.util.download import download
 from compiler_gym.util.filesystem import atomic_file_write
 
+logger = logging.getLogger(__name__)
+
 # Module-level locks that ensures exclusive access to install routines across
 # threads. Note that these lock are shared across all TarDataset instances. We
 # don't use per-dataset locks as locks cannot be pickled.
@@ -89,9 +92,12 @@ def install(self) -> None:
             # Remove any partially-completed prior extraction.
             shutil.rmtree(self.site_data_path / "contents", ignore_errors=True)
 
-            self.logger.info("Downloading %s dataset", self.name)
+            logger.warning(
+                "Installing the %s dataset. This may take a few moments ...", self.name
+            )
+
             tar_data = io.BytesIO(download(self.tar_urls, self.tar_sha256))
-            self.logger.info("Unpacking %s dataset", self.name)
+            logger.info("Unpacking %s dataset to %s", self.name, self.site_data_path)
             with tarfile.open(
                 fileobj=tar_data, mode=f"r:{self.tar_compression}"
             ) as arc:
@@ -165,7 +171,7 @@ def _read_manifest_file(self) -> List[str]:
         """
         with open(self._manifest_path, encoding="utf-8") as f:
             uris = self._read_manifest(f.read())
-        self.logger.debug("Read %s manifest, %d entries", self.name, len(uris))
+        logger.debug("Read %s manifest, %d entries", self.name, len(uris))
         return uris
 
     @memoized_property
@@ -192,7 +198,7 @@ def _benchmark_uris(self) -> List[str]:
                 )
 
             # Decompress the manifest data.
-            self.logger.debug("Downloading %s manifest", self.name)
+            logger.debug("Downloading %s manifest", self.name)
             manifest_data = io.BytesIO(
                 download(self.manifest_urls, self.manifest_sha256)
             )
@@ -206,9 +212,7 @@ def _benchmark_uris(self) -> List[str]:
                 f.write(manifest_data)
 
             uris = self._read_manifest(manifest_data.decode("utf-8"))
-            self.logger.debug(
-                "Downloaded %s manifest, %d entries", self.name, len(uris)
-            )
+            logger.debug("Downloaded %s manifest, %d entries", self.name, len(uris))
             return uris
 
     @memoized_property

compiler_gym/envs/compiler_env.py

@@ -47,7 +47,6 @@
     proto_to_action_space,
 )
 from compiler_gym.spaces import DefaultRewardFromObservation, NamedDiscrete, Reward
-from compiler_gym.util.debug_util import get_logging_level
 from compiler_gym.util.gym_type_hints import (
     ActionType,
     ObservationType,
@@ -60,6 +59,13 @@
 from compiler_gym.validation_result import ValidationResult
 from compiler_gym.views import ObservationSpaceSpec, ObservationView, RewardView
 
+logger = logging.getLogger(__name__)
+
+# NOTE(cummins): This is only required to prevent a name conflict with the now
+# deprecated CompilerEnv.logger attribute. This can be removed once the logger
+# attribute is removed, scheduled for release 0.2.3.
+_logger = logger
+
 
 def _wrapped_step(
     service: CompilerGymServiceConnection, request: StepRequest
@@ -113,11 +119,6 @@ class CompilerEnv(gym.Env):
 
     :vartype service: compiler_gym.service.CompilerGymServiceConnection
 
-    :ivar logger: A Logger instance used by the environment for communicating
-        info and warnings.
-
-    :vartype logger: logging.Logger
-
     :ivar action_spaces: A list of supported action space names.
 
     :vartype action_spaces: List[str]
@@ -210,23 +211,23 @@ def __init__(
         :param service_connection: An existing compiler gym service connection
             to use.
 
-        :param logger: The logger to use for this environment. If not provided,
-            a :code:`compiler_gym.envs` logger is used and assigned the
-            verbosity returned by :func:`get_logging_level()
-            <compiler_gym.get_logging_level>`.
-
         :raises FileNotFoundError: If service is a path to a file that is not
             found.
 
         :raises TimeoutError: If the compiler service fails to initialize within
             the parameters provided in :code:`connection_settings`.
         """
-        self.metadata = {"render.modes": ["human", "ansi"]}
+        # NOTE(cummins): Logger argument deprecated and scheduled to be removed
+        # in release 0.2.3.
+        if logger:
+            warnings.warn(
+                "The `logger` argument is deprecated on CompilerEnv.__init__() "
+                "and will be removed in a future release. All CompilerEnv "
+                "instances share a logger named compiler_gym.envs.compiler_env",
+                DeprecationWarning,
+            )
 
-        if logger is None:
-            logger = logging.getLogger("compiler_gym.envs")
-            logger.setLevel(get_logging_level())
-        self.logger = logger
+        self.metadata = {"render.modes": ["human", "ansi"]}
 
         # A compiler service supports multiple simultaneous environments. This
         # session ID is used to identify this environment.
@@ -238,7 +239,6 @@ def __init__(
         self.service = service_connection or CompilerGymServiceConnection(
             endpoint=self._service_endpoint,
             opts=self._connection_settings,
-            logger=self.logger,
         )
         self.datasets = Datasets(datasets or [])
 
@@ -330,6 +330,17 @@ def available_datasets(self) -> Dict[str, Dataset]:
         """A dictionary of datasets."""
         return {d.name: d for d in self.datasets}
 
+    @property
+    @deprecated(
+        version="0.2.1",
+        reason=(
+            "The `CompilerEnv.logger` attribute is deprecated. All CompilerEnv "
+            "instances share a logger named compiler_gym.envs.compiler_env"
+        ),
+    )
+    def logger(self):
+        return _logger
+
     @property
     def versions(self) -> GetVersionReply:
         """Get the version numbers from the compiler service."""
@@ -447,10 +458,10 @@ def benchmark(self, benchmark: Union[str, Benchmark]):
             )
         if isinstance(benchmark, str):
             benchmark_object = self.datasets.benchmark(benchmark)
-            self.logger.debug("Setting benchmark by name: %s", benchmark_object)
+            logger.debug("Setting benchmark by name: %s", benchmark_object)
             self._next_benchmark = benchmark_object
         elif isinstance(benchmark, Benchmark):
-            self.logger.debug("Setting benchmark: %s", benchmark.uri)
+            logger.debug("Setting benchmark: %s", benchmark.uri)
             self._next_benchmark = benchmark
         else:
             raise TypeError(
@@ -573,9 +584,7 @@ def fork(self) -> "CompilerEnv":
             actions = self.actions.copy()
             self.reset()
             if actions:
-                self.logger.warning(
-                    "Parent service of fork() has died, replaying state"
-                )
+                logger.warning("Parent service of fork() has died, replaying state")
                 _, _, done, _ = self.step(actions)
                 assert not done, "Failed to replay action sequence"
 
@@ -668,7 +677,7 @@ def close(self):
                 if reply.remaining_sessions:
                     close_service = False
             except Exception as e:
-                self.logger.warning(
+                logger.warning(
                     "Failed to end active compiler session on close(): %s (%s)",
                     e,
                     type(e).__name__,
@@ -720,7 +729,7 @@ def reset(  # pylint: disable=arguments-differ
 
         def _retry(error) -> Optional[ObservationType]:
             """Abort and retry on error."""
-            self.logger.warning("%s during reset(): %s", type(error).__name__, error)
+            logger.warning("%s during reset(): %s", type(error).__name__, error)
             if self.service:
                 self.service.close()
             self.service = None
@@ -763,13 +772,13 @@ def _call_with_error(
 
         # Stop an existing episode.
         if self.in_episode:
-            self.logger.debug("Ending session %d", self._session_id)
+            logger.debug("Ending session %d", self._session_id)
             error, _ = _call_with_error(
                 self.service.stub.EndSession,
                 EndSessionRequest(session_id=self._session_id),
             )
             if error:
-                self.logger.warning(
+                logger.warning(
                     "Failed to stop session %d with %s: %s",
                     self._session_id,
                     type(error).__name__,