From b1aa2e577e67a54c3ca3b6c0feb8bf950b886990 Mon Sep 17 00:00:00 2001 From: Chris Cummins Date: Mon, 7 Mar 2022 09:42:39 +0000 Subject: [PATCH] Add backwards compatibility for Reward.id. We don't want the change introduced in #565 to immediately break existing user code. Instead, add a workaround that emits a deprecation warning so that users can update their code upon the next release. Issue #381. --- compiler_gym/spaces/reward.py | 44 ++++++++++++++++++++++++++++------- tests/spaces/BUILD | 10 ++++++++ tests/spaces/CMakeLists.txt | 10 ++++++++ tests/spaces/reward_test.py | 25 ++++++++++++++++++++ 4 files changed, 81 insertions(+), 8 deletions(-) create mode 100644 tests/spaces/reward_test.py diff --git a/compiler_gym/spaces/reward.py b/compiler_gym/spaces/reward.py index a5952d1938..cccc2169d9 100644 --- a/compiler_gym/spaces/reward.py +++ b/compiler_gym/spaces/reward.py @@ -2,6 +2,7 @@ # # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. +import warnings from typing import List, Optional, Tuple, Union import numpy as np @@ -38,7 +39,10 @@ class Reward(Scalar): def __init__( self, - name: str, + # NOTE(github.com/facebookresearch/CompilerGym/issues/381): Once `id` + # argument has been removed, the default value for `name` can be + # removed. + name: str = None, observation_spaces: Optional[List[str]] = None, default_value: RewardType = 0, min: Optional[RewardType] = None, @@ -47,6 +51,10 @@ def __init__( success_threshold: Optional[RewardType] = None, deterministic: bool = False, platform_dependent: bool = True, + # NOTE(github.com/facebookresearch/CompilerGym/issues/381): Backwards + # compatability workaround for deprecated parameter, will be removed in + # v0.2.4. + id: Optional[str] = None, ): """Constructor. @@ -56,18 +64,18 @@ def __init__( (:class:`space.id ` values) that are used to compute the reward. May be an empty list if no observations are requested. Requested observations will be provided - to the :code:`observations` argument of - :meth:`reward.update() `. + to the :code:`observations` argument of :meth:`reward.update() + `. :param default_value: A default reward. This value will be returned by - :meth:`env.step() ` if - the service terminates. + :meth:`env.step() ` if the + service terminates. :param min: The lower bound of the reward. :param max: The upper bound of the reward. :param default_negates_returns: If true, the default value will be offset by the sum of all rewards for the current episode. For example, given a default reward value of *-10.0* and an episode with - prior rewards *[0.1, 0.3, -0.15]*, the default value is: - *-10.0 - sum(0.1, 0.3, -0.15)*. + prior rewards *[0.1, 0.3, -0.15]*, the default value is: *-10.0 - + sum(0.1, 0.3, -0.15)*. :param success_threshold: The cumulative reward threshold before an episode is considered successful. For example, episodes where reward is scaled to an existing heuristic can be considered “successful” @@ -75,6 +83,10 @@ def __init__( :param deterministic: Whether the reward space is deterministic. :param platform_dependent: Whether the reward values depend on the execution environment of the service. + :param id: The name of the reward space. + + .. deprecated:: 0.2.3 + Use :code:`name` instead. """ super().__init__( name=name, @@ -82,7 +94,23 @@ def __init__( max=np.inf if max is None else max, dtype=np.float64, ) - self.name = name + + # NOTE(github.com/facebookresearch/CompilerGym/issues/381): Backwards + # compatability workaround for deprecated parameter, will be removed in + # v0.2.4. + if id is not None: + warnings.warn( + "The `id` argument of " + "compiler_gym.spaces.Reward.__init__() " + "has been renamed `name`. This will break in a future release, " + "please update your code.", + DeprecationWarning, + ) + self.name = name or id + self.id = self.name + if not self.name: + raise TypeError("No name given") + self.observation_spaces = observation_spaces or [] self.default_value: RewardType = default_value self.default_negates_returns: bool = default_negates_returns diff --git a/tests/spaces/BUILD b/tests/spaces/BUILD index e4a714c79b..678e738780 100644 --- a/tests/spaces/BUILD +++ b/tests/spaces/BUILD @@ -24,6 +24,16 @@ py_test( ], ) +py_test( + name = "reward_test", + timeout = "short", + srcs = ["reward_test.py"], + deps = [ + "//compiler_gym/spaces", + "//tests:test_main", + ], +) + py_test( name = "scalar_test", timeout = "short", diff --git a/tests/spaces/CMakeLists.txt b/tests/spaces/CMakeLists.txt index d80905b624..1231328b9b 100644 --- a/tests/spaces/CMakeLists.txt +++ b/tests/spaces/CMakeLists.txt @@ -25,6 +25,16 @@ cg_py_test( tests::test_main ) +cg_py_test( + NAME + reward_test + SRCS + "reward_test.py" + DEPS + compiler_gym::spaces::spaces + tests::test_main +) + cg_py_test( NAME scalar_test diff --git a/tests/spaces/reward_test.py b/tests/spaces/reward_test.py new file mode 100644 index 0000000000..c0f6384d72 --- /dev/null +++ b/tests/spaces/reward_test.py @@ -0,0 +1,25 @@ +# Copyright (c) Facebook, Inc. and its affiliates. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. +"""Unit tests for compiler_gym.spaces.Reward.""" +import pytest + +from compiler_gym.spaces import Reward +from tests.test_main import main + + +def test_reward_id_backwards_compatibility(): + """Test that Reward.id is backwards compatible with Reward.name. + + See: github.com/facebookresearch/CompilerGym/issues/381 + """ + with pytest.warns(DeprecationWarning, match="renamed `name`"): + reward = Reward(id="foo") + + assert reward.id == "foo" + assert reward.name == "foo" + + +if __name__ == "__main__": + main()