From ae9a3a22376aa6621ce90f1b1234c8942831f899 Mon Sep 17 00:00:00 2001 From: Sven Mika Date: Thu, 23 Jan 2020 02:02:58 +0100 Subject: [PATCH] [RLlib] from_config util method for framework agnostic components; start moving RLlib tests into Bazel. (#6865) --- ci/jenkins_tests/run_rllib_tests.sh | 6 - ci/travis/install-dependencies.sh | 4 +- python/ray/tune/utils/util.py | 9 +- rllib/BUILD | 43 ++++ rllib/agents/a3c/a3c_torch_policy.py | 3 +- rllib/agents/qmix/mixers.py | 3 +- rllib/agents/qmix/model.py | 3 +- rllib/utils/__init__.py | 28 +++ rllib/utils/from_config.py | 215 ++++++++++++++++++ rllib/utils/test_utils.py | 22 +- rllib/utils/tests/__init__.py | 0 rllib/utils/tests/dummy_config.json | 6 + rllib/utils/tests/dummy_config.yml | 5 + .../test_framework_agnostic_components.py | 104 +++++++++ 14 files changed, 430 insertions(+), 21 deletions(-) create mode 100644 rllib/BUILD create mode 100644 rllib/utils/from_config.py create mode 100644 rllib/utils/tests/__init__.py create mode 100644 rllib/utils/tests/dummy_config.json create mode 100644 rllib/utils/tests/dummy_config.yml create mode 100644 rllib/utils/tests/test_framework_agnostic_components.py diff --git a/ci/jenkins_tests/run_rllib_tests.sh b/ci/jenkins_tests/run_rllib_tests.sh index 98cbd0c83..bec1632de 100755 --- a/ci/jenkins_tests/run_rllib_tests.sh +++ b/ci/jenkins_tests/run_rllib_tests.sh @@ -34,9 +34,6 @@ docker run --rm --shm-size=${SHM_SIZE} --memory=${MEMORY_SIZE} $DOCKER_SHA \ --stop '{"training_iteration": 1}' \ --config '{"num_workers": 2}' -docker run --rm --shm-size=${SHM_SIZE} --memory=${MEMORY_SIZE} $DOCKER_SHA \ - /ray/ci/suppress_output python /ray/rllib/agents/ppo/tests/test_ppo.py - docker run --rm --shm-size=${SHM_SIZE} --memory=${MEMORY_SIZE} $DOCKER_SHA \ /ray/ci/suppress_output /ray/rllib/train.py \ --env CartPole-v1 \ @@ -168,9 +165,6 @@ docker run --rm --shm-size=${SHM_SIZE} --memory=${MEMORY_SIZE} $DOCKER_SHA \ --stop '{"training_iteration": 1}' \ --config '{"num_workers": 2}' -docker run --rm --shm-size=${SHM_SIZE} --memory=${MEMORY_SIZE} $DOCKER_SHA \ - /ray/ci/suppress_output python /ray/rllib/agents/pg/tests/test_pg.py - docker run --rm --shm-size=${SHM_SIZE} --memory=${MEMORY_SIZE} $DOCKER_SHA \ /ray/ci/suppress_output /ray/rllib/train.py \ --env CartPole-v0 \ diff --git a/ci/travis/install-dependencies.sh b/ci/travis/install-dependencies.sh index e22a6fabe..24550ab3e 100755 --- a/ci/travis/install-dependencies.sh +++ b/ci/travis/install-dependencies.sh @@ -27,7 +27,7 @@ if [[ "$PYTHON" == "3.5" ]] && [[ "$platform" == "linux" ]]; then pip install -q scipy tensorflow cython==0.29.0 gym opencv-python-headless pyyaml pandas==0.24.2 requests \ feather-format lxml openpyxl xlrd py-spy setproctitle pytest-timeout networkx tabulate psutil aiohttp \ uvicorn dataclasses pygments werkzeug kubernetes flask grpcio pytest-sugar pytest-rerunfailures pytest-asyncio \ - blist + blist torch torchvision elif [[ "$PYTHON" == "3.5" ]] && [[ "$platform" == "macosx" ]]; then # Install miniconda. wget -q https://repo.continuum.io/miniconda/Miniconda3-4.5.4-MacOSX-x86_64.sh -O miniconda.sh -nv @@ -36,7 +36,7 @@ elif [[ "$PYTHON" == "3.5" ]] && [[ "$platform" == "macosx" ]]; then pip install -q cython==0.29.0 tensorflow gym opencv-python-headless pyyaml pandas==0.24.2 requests \ feather-format lxml openpyxl xlrd py-spy setproctitle pytest-timeout networkx tabulate psutil aiohttp \ uvicorn dataclasses pygments werkzeug kubernetes flask grpcio pytest-sugar pytest-rerunfailures pytest-asyncio \ - blist + blist torch torchvision elif [[ "$LINT" == "1" ]]; then sudo apt-get update sudo apt-get install -y build-essential curl unzip diff --git a/python/ray/tune/utils/util.py b/python/ray/tune/utils/util.py index d68646edb..943592737 100644 --- a/python/ray/tune/utils/util.py +++ b/python/ray/tune/utils/util.py @@ -137,7 +137,14 @@ class warn_if_slow: def merge_dicts(d1, d2): - """Returns a new dict that is d1 and d2 deep merged.""" + """ + Args: + d1 (dict): Dict 1. + d2 (dict): Dict 2. + + Returns: + dict: A new dict that is d1 and d2 deep merged. + """ merged = copy.deepcopy(d1) deep_update(merged, d2, True, []) return merged diff --git a/rllib/BUILD b/rllib/BUILD new file mode 100644 index 000000000..d32c0d459 --- /dev/null +++ b/rllib/BUILD @@ -0,0 +1,43 @@ +# -------------------------------------------------------------------- +# BAZEL/Travis-ci test cases. +# +# NOTE: Move more test cases here from ci/jenkins/run_rllib_tests.sh +# as Travis tests seem to run more stable. +# -------------------------------------------------------------------- + + +# --------------------------------------- +# Agents (short learning tasks) +# --------------------------------------- + +# TODO: + +# --------------------------------------- +# Agents (Compilation and Losses) +# --------------------------------------- + +# PGTrainer +py_test( + name = "test_pg", + size = "small", + srcs = ["agents/pg/tests/test_pg.py"] +) + +# PPOTrainer +py_test( + name = "test_ppo", + size = "small", + srcs = ["agents/ppo/tests/test_ppo.py", + "agents/ppo/tests/test.py"] # TODO: Move down once PR 6889 merged +) + +# --------------------------------------- +# Models and Distributions +# --------------------------------------- + +# TODO: Move here once PR 6889 merged +#py_test( +# name = "test_distributions", +# size = "small", +# srcs = ["models/tests/test_distributions.py"] +#) diff --git a/rllib/agents/a3c/a3c_torch_policy.py b/rllib/agents/a3c/a3c_torch_policy.py index d4acaea74..0581ad8d6 100644 --- a/rllib/agents/a3c/a3c_torch_policy.py +++ b/rllib/agents/a3c/a3c_torch_policy.py @@ -6,7 +6,6 @@ from ray.rllib.policy.torch_policy_template import build_torch_policy from ray.rllib.utils.framework import try_import_torch torch, nn = try_import_torch() -F = nn.functional def actor_critic_loss(policy, model, dist_class, train_batch): @@ -17,7 +16,7 @@ def actor_critic_loss(policy, model, dist_class, train_batch): policy.entropy = dist.entropy().mean() policy.pi_err = -train_batch[Postprocessing.ADVANTAGES].dot( log_probs.reshape(-1)) - policy.value_err = F.mse_loss( + policy.value_err = nn.functional.mse_loss( values.reshape(-1), train_batch[Postprocessing.VALUE_TARGETS]) overall_err = sum([ policy.pi_err, diff --git a/rllib/agents/qmix/mixers.py b/rllib/agents/qmix/mixers.py index 3e05a0dba..7d790d797 100644 --- a/rllib/agents/qmix/mixers.py +++ b/rllib/agents/qmix/mixers.py @@ -3,7 +3,6 @@ import numpy as np from ray.rllib.utils.framework import try_import_torch torch, nn = try_import_torch() -F = nn.functional class VDNMixer(nn.Module): @@ -49,7 +48,7 @@ class QMixer(nn.Module): b1 = self.hyper_b_1(states) w1 = w1.view(-1, self.n_agents, self.embed_dim) b1 = b1.view(-1, 1, self.embed_dim) - hidden = F.elu(torch.bmm(agent_qs, w1) + b1) + hidden = nn.functional.elu(torch.bmm(agent_qs, w1) + b1) # Second layer w_final = torch.abs(self.hyper_w_final(states)) w_final = w_final.view(-1, self.embed_dim, 1) diff --git a/rllib/agents/qmix/model.py b/rllib/agents/qmix/model.py index 41d4d01d6..905533302 100644 --- a/rllib/agents/qmix/model.py +++ b/rllib/agents/qmix/model.py @@ -4,7 +4,6 @@ from ray.rllib.utils.annotations import override from ray.rllib.utils import try_import_torch torch, nn = try_import_torch() -F = nn.functional class RNNModel(TorchModelV2, nn.Module): @@ -28,7 +27,7 @@ class RNNModel(TorchModelV2, nn.Module): @override(TorchModelV2) def forward(self, input_dict, hidden_state, seq_lens): - x = F.relu(self.fc1(input_dict["obs_flat"].float())) + x = nn.functional.relu(self.fc1(input_dict["obs_flat"].float())) h_in = hidden_state[0].reshape(-1, self.rnn_hidden_dim) h = self.rnn(x, h_in) q = self.fc2(h) diff --git a/rllib/utils/__init__.py b/rllib/utils/__init__.py index 07bed46e6..2600ab61f 100644 --- a/rllib/utils/__init__.py +++ b/rllib/utils/__init__.py @@ -1,3 +1,5 @@ +from functools import partial + from ray.rllib.utils.annotations import override, PublicAPI, DeveloperAPI from ray.rllib.utils.framework import try_import_tf, try_import_tfp, \ try_import_torch @@ -28,11 +30,37 @@ def add_mixins(base, mixins): return base +def force_list(elements=None, to_tuple=False): + """ + Makes sure `elements` is returned as a list, whether `elements` is a single + item, already a list, or a tuple. + + Args: + elements (Optional[any]): The inputs as single item, list, or tuple to + be converted into a list/tuple. If None, returns empty list/tuple. + to_tuple (bool): Whether to use tuple (instead of list). + + Returns: + Union[list,tuple]: All given elements in a list/tuple depending on + `to_tuple`'s value. If elements is None, + returns an empty list/tuple. + """ + ctor = list + if to_tuple is True: + ctor = tuple + return ctor() if elements is None else ctor(elements) \ + if type(elements) in [list, tuple] else ctor([elements]) + + +force_tuple = partial(force_list, to_tuple=True) + __all__ = [ "add_mixins", "check", "deprecation_warning", "fc", + "force_list", + "force_tuple", "lstm", "one_hot", "relu", diff --git a/rllib/utils/from_config.py b/rllib/utils/from_config.py new file mode 100644 index 000000000..b8ace5652 --- /dev/null +++ b/rllib/utils/from_config.py @@ -0,0 +1,215 @@ +from copy import deepcopy +from functools import partial +import importlib +import json +import os +import re +import yaml + +from ray.rllib.utils import force_list, merge_dicts + + +def from_config(cls, config=None, **kwargs): + """ + Uses the given config to create an object. + If `config` is a dict, an optional "type" key can be used as a + "constructor hint" to specify a certain class of the object. + If `config` is not a dict, `config`'s value is used directly as this + "constructor hint". + + The rest of `config` (if it's a dict) will be used as kwargs for the + constructor. Additional keys in **kwargs will always have precedence + (overwrite keys in `config` (if a dict)). + Also, if the config-dict or **kwargs contains the special key "_args", + it will be popped from the dict and used as *args list to be passed + separately to the constructor. + + The following constructor hints are valid: + - None: Use `cls` as constructor. + - An already instantiated object: Will be returned as is; no + constructor call. + - A string or an object that is a key in `cls`'s `__type_registry__` + dict: The value in `__type_registry__` for that key will be used + as the constructor. + - A python callable: Use that very callable as constructor. + - A string: Either a json/yaml filename or the name of a python + module+class (e.g. "ray.rllib. [...] .[some class name]") + + Args: + cls (class): The class to build an instance for (from `config`). + config (Optional[dict,str]): The config dict or type-string or + filename. + + Keyword Args: + kwargs (any): Optional possibility to pass the c'tor arguments in + here and use `config` as the type-only info. Then we can call + this like: from_config([type]?, [**kwargs for c'tor]) + If `config` is already a dict, then `kwargs` will be merged + with `config` (overwriting keys in `config`) after "type" has + been popped out of `config`. + If a constructor of a Configurable needs *args, the special + key `_args` can be passed inside `kwargs` with a list value + (e.g. kwargs={"_args": [arg1, arg2, arg3]}). + + Returns: + any: The object generated from the config. + """ + # `cls` is the config (config is None). + if config is None and isinstance(cls, (dict, str)): + config = cls + cls = None + # `config` is already a created object of this class -> + # Take it as is. + elif isinstance(cls, type) and isinstance(config, cls): + return config + + # `type_`: Indicator for the Configurable's constructor. + # `ctor_args`: *args arguments for the constructor. + # `ctor_kwargs`: **kwargs arguments for the constructor. + # Try to copy, so caller can reuse safely. + try: + config = deepcopy(config) + except Exception: + pass + if isinstance(config, dict): + type_ = config.pop("type", None) + ctor_kwargs = config + # Give kwargs priority over things defined in config dict. + # This way, one can pass a generic `spec` and then override single + # c'tor parameters via the kwargs in the call to `from_config`. + ctor_kwargs.update(kwargs) + else: + type_ = config + if type_ is None and "type" in kwargs: + type_ = kwargs.pop("type") + ctor_kwargs = kwargs + # Special `_args` field in kwargs for *args-utilizing constructors. + ctor_args = force_list(ctor_kwargs.pop("_args", [])) + + # Figure out the actual constructor (class) from `type_`. + # None: Try __default__object (if no args/kwargs), only then + # constructor of cls (using args/kwargs). + if type_ is None: + # We have a default constructor that was defined directly by cls + # (not by its children). + if cls is not None and cls.__default_constructor__ is not None and \ + ctor_args == [] and \ + ( + not hasattr(cls.__bases__[0], "__default_constructor__") + or + cls.__bases__[0].__default_constructor__ is None or + cls.__bases__[0].__default_constructor__ is not + cls.__default_constructor__ + ): + constructor = cls.__default_constructor__ + # Default constructor's keywords into ctor_kwargs. + if isinstance(constructor, partial): + kwargs = merge_dicts(ctor_kwargs, constructor.keywords) + constructor = partial(constructor.func, **kwargs) + ctor_kwargs = {} # erase to avoid duplicate kwarg error + # No default constructor -> Try cls itself as c'tor. + else: + constructor = cls + # Try the __type_registry__ of this class. + else: + constructor = lookup_type(cls, type_) + + # Found in cls.__type_registry__. + if constructor is not None: + pass + # type_ is False or None (and this value is not registered) -> + # return value of type_. + elif type_ is False or type_ is None: + return type_ + # Python callable. + elif callable(type_): + constructor = type_ + # A string: Filename or a python module+class or a json/yaml str. + elif isinstance(type_, str): + if re.search("\.(yaml|yml|json)$", type_): + return from_file(cls, type_, *ctor_args, **ctor_kwargs) + # Try un-json/un-yaml'ing the string into a dict. + obj = yaml.load(type_) + if isinstance(obj, dict): + return from_config(cls, obj) + try: + obj = from_config(cls, json.loads(type_)) + except json.JSONDecodeError: + pass + else: + return obj + + if type_.find(".") != -1: + module_name, function_name = type_.rsplit(".", 1) + try: + module = importlib.import_module(module_name) + constructor = getattr(module, function_name) + except (ModuleNotFoundError, ImportError): + pass + if constructor is None: + raise ValueError( + "String specifier ({}) in `from_config` must be a " + "filename, a module+class, or a key into " + "{}.__type_registry__!".format(type_, cls.__name__)) + + if not constructor: + raise TypeError( + "Invalid type '{}'. Cannot create `from_config`.".format(type_)) + + # Create object with inferred constructor. + try: + object_ = constructor(*ctor_args, **ctor_kwargs) + # Catch attempts to construct from an abstract class and return None. + except TypeError as e: + if re.match("Can't instantiate abstract class", e.args[0]): + return None + raise e # Re-raise + # No sanity check for fake (lambda)-"constructors". + if type(constructor).__name__ != "function": + assert isinstance( + object_, constructor.func + if isinstance(constructor, partial) else constructor) + + return object_ + + +def from_file(cls, filename, *args, **kwargs): + """ + Create object from config saved in filename. Expects json or yaml file. + + Args: + filename (str): File containing the config (json or yaml). + + Returns: + any: The object generated from the file. + """ + path = os.path.join(os.getcwd(), filename) + if not os.path.isfile(path): + raise FileNotFoundError("File '{}' not found!".format(filename)) + + with open(path, "rt") as fp: + if path.endswith(".yaml") or path.endswith(".yml"): + config = yaml.load(fp) + else: + config = json.load(fp) + + # Add possible *args. + config["_args"] = args + return from_config(cls, config=config, **kwargs) + + +def lookup_type(cls, type_): + if cls is not None and isinstance(cls.__type_registry__, dict) and \ + ( + type_ in cls.__type_registry__ or ( + isinstance(type_, str) and + re.sub("[\W_]", "", type_.lower()) + in cls.__type_registry__ + ) + ): + available_class_for_type = cls.__type_registry__.get(type_) + if available_class_for_type is None: + available_class_for_type = \ + cls.__type_registry__[re.sub("[\W_]", "", type_.lower())] + return available_class_for_type + return None diff --git a/rllib/utils/test_utils.py b/rllib/utils/test_utils.py index 8759296ce..fc6aa2e95 100644 --- a/rllib/utils/test_utils.py +++ b/rllib/utils/test_utils.py @@ -31,8 +31,13 @@ def check(x, y, decimals=5, atol=None, rtol=None, false=False): for key, value in x.items(): assert key in y, \ "ERROR: y does not have x's key='{}'! y={}".format(key, y) - check(value, y[key], decimals=decimals, atol=atol, rtol=rtol, - false=false) + check( + value, + y[key], + decimals=decimals, + atol=atol, + rtol=rtol, + false=false) y_keys.remove(key) assert not y_keys, \ "ERROR: y contains keys ({}) that are not in x! y={}".\ @@ -45,8 +50,13 @@ def check(x, y, decimals=5, atol=None, rtol=None, false=False): "ERROR: y does not have the same length as x ({} vs {})!".\ format(len(y), len(x)) for i, value in enumerate(x): - check(value, y[i], decimals=decimals, atol=atol, rtol=rtol, - false=false) + check( + value, + y[i], + decimals=decimals, + atol=atol, + rtol=rtol, + false=false) # Boolean comparison. elif isinstance(x, (np.bool_, bool)): if false is True: @@ -55,8 +65,8 @@ def check(x, y, decimals=5, atol=None, rtol=None, false=False): else: assert bool(x) is bool(y), \ "ERROR: x ({}) is not y ({})!".format(x, y) - # Nones. - elif x is None or y is None: + # Nones or primitives. + elif x is None or y is None or isinstance(x, (str, int, float)): if false is True: assert x != y, "ERROR: x ({}) is the same as y ({})!".format(x, y) else: diff --git a/rllib/utils/tests/__init__.py b/rllib/utils/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/rllib/utils/tests/dummy_config.json b/rllib/utils/tests/dummy_config.json new file mode 100644 index 000000000..b1a0bb653 --- /dev/null +++ b/rllib/utils/tests/dummy_config.json @@ -0,0 +1,6 @@ +{ + "type": "ray.rllib.utils.tests.test_framework_agnostic_components.DummyComponent", + "prop_a": "some-value", + "prop_b": 2.0, + "some_other_kwarg": "this_is_a_component_from_json_file" +} diff --git a/rllib/utils/tests/dummy_config.yml b/rllib/utils/tests/dummy_config.yml new file mode 100644 index 000000000..074866f0b --- /dev/null +++ b/rllib/utils/tests/dummy_config.yml @@ -0,0 +1,5 @@ +type: ray.rllib.utils.tests.test_framework_agnostic_components.DummyComponent +prop_a: something else +prop_b: 1.0 +prop_d: 3 +framework: torch diff --git a/rllib/utils/tests/test_framework_agnostic_components.py b/rllib/utils/tests/test_framework_agnostic_components.py new file mode 100644 index 000000000..1cf7a1ea0 --- /dev/null +++ b/rllib/utils/tests/test_framework_agnostic_components.py @@ -0,0 +1,104 @@ +from abc import ABCMeta, abstractmethod +import unittest + +from ray.rllib.utils.from_config import from_config +from ray.rllib.utils.test_utils import check +from ray.rllib.utils.framework import try_import_tf, try_import_torch + +tf = try_import_tf() +tf.enable_eager_execution() + +torch, _ = try_import_torch() + + +class TestFrameWorkAgnosticComponents(unittest.TestCase): + """ + Tests the Component base class to implement framework-agnostic functional + units. + """ + + def test_dummy_components(self): + # Switch on eager for testing purposes. + tf.enable_eager_execution() + + # Try to create from an abstract class w/o default constructor. + # Expect None. + test = from_config({ + "type": AbstractDummyComponent, + "framework": "torch" + }) + check(test, None) + + # Create a Component via python API (config dict). + component = from_config( + dict(type=DummyComponent, prop_a=1.0, prop_d="non_default")) + check(component.prop_d, "non_default") + + # Create a tf Component from json file. + component = from_config("dummy_config.json") + check(component.prop_c, "default") + check(component.prop_d, 4) # default + check(component.add(3.3).numpy(), 5.3) # prop_b == 2.0 + + # Create a torch Component from yaml file. + component = from_config("dummy_config.yml") + check(component.prop_a, "something else") + check(component.prop_d, 3) + check(component.add(1.2), torch.Tensor([2.2])) # prop_b == 1.0 + + # Create tf Component from json-string (e.g. on command line). + component = from_config( + '{"type": "ray.rllib.utils.tests.' + 'test_framework_agnostic_components.DummyComponent", ' + '"prop_a": "A", "prop_b": -1.0, "prop_c": "non-default"}') + check(component.prop_a, "A") + check(component.prop_d, 4) # default + check(component.add(-1.1).numpy(), -2.1) # prop_b == -1.0 + + # Create torch Component from yaml-string. + component = from_config( + "type: ray.rllib.utils.tests." + "test_framework_agnostic_components.DummyComponent\n" + "prop_a: B\nprop_b: -1.5\nprop_c: non-default\nframework: torch") + check(component.prop_a, "B") + check(component.prop_d, 4) # default + check(component.add(-5.1), torch.Tensor([-6.6])) # prop_b == -1.5 + + +class DummyComponent: + """ + A simple DummyComponent that can be used for testing framework-agnostic + logic. Implements a simple `add()` method for adding a value to + `self.prop_b`. + """ + + def __init__(self, + prop_a, + prop_b=0.5, + prop_c=None, + framework="tf", + **kwargs): + self.framework = framework + self.prop_a = prop_a + self.prop_b = prop_b + self.prop_c = prop_c or "default" + self.prop_d = kwargs.pop("prop_d", 4) + self.kwargs = kwargs + + def add(self, value): + if self.framework == "tf": + return self._add_tf(value) + return self.prop_b + value + + def _add_tf(self, value): + return tf.add(self.prop_b, value) + + +class AbstractDummyComponent(DummyComponent, metaclass=ABCMeta): + """ + Used for testing `from_config()`. + """ + + @abstractmethod + def some_abstract_method(self): + raise NotImplementedError