Skip to content

Commit

Permalink
Fix the TunableGroups.is_updated() logic (#452)
Browse files Browse the repository at this point in the history
Summary of changes:
* Set the `CovariantTunableGroup._is_updated` flag *only* when assigning
its tunables the data that is different from the tunables' current
values.
* Add unit tests to verify the new behavior of the `TunableGroups`.
* In the `Environment.setup()` method, write to the log if the
environment has to be reset due to the update to its tunables.

We will update the logic of the derived `Environment` classes in the
subsequent PRs.
  • Loading branch information
motus authored Jul 19, 2023
1 parent 4a23149 commit eb6205d
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 3 deletions.
13 changes: 13 additions & 0 deletions mlos_bench/mlos_bench/environments/base_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,22 @@ def setup(self, tunables: TunableGroups, global_config: Optional[dict] = None) -
"""
_LOG.info("Setup %s :: %s", self, tunables)
assert isinstance(tunables, TunableGroups)

# Assign new values to the environment's tunable parameters:
groups = list(self._tunable_params.get_covariant_group_names())
self._tunable_params.assign(tunables.get_param_values(groups))

# Write to the log whether the environment needs to be reset.
# (Derived classes still have to check `self._tunable_params.is_updated()`).
is_updated = self._tunable_params.is_updated()
if _LOG.isEnabledFor(logging.DEBUG):
_LOG.debug("Env '%s': Tunable groups reset = %s :: %s", self, is_updated, {
name: self._tunable_params.is_updated([name])
for name in self._tunable_params.get_covariant_group_names()
})
else:
_LOG.info("Env '%s': Tunable groups reset = %s", self, is_updated)

# Combine tunables, const_args, and global config into `self._params`:
self._params = self._combine_tunables(tunables)
merge_parameters(dest=self._params, source=global_config)
Expand Down
61 changes: 61 additions & 0 deletions mlos_bench/mlos_bench/tests/tunables/tunable_group_update_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
#
"""
Tests for checking the is_updated flag for tunable groups.
"""

from mlos_bench.tunables.tunable_groups import TunableGroups

_TUNABLE_VALUES = {
"kernel_sched_migration_cost_ns": 8888,
"kernel_sched_latency_ns": 9999,
}


def test_tunable_group_update(tunable_groups: TunableGroups) -> None:
"""
Test that updating a tunable group raises the is_updated flag.
"""
tunable_groups.assign(_TUNABLE_VALUES)
assert tunable_groups.is_updated()


def test_tunable_group_update_twice(tunable_groups: TunableGroups) -> None:
"""
Test that updating a tunable group with the same values do *NOT* raises the is_updated flag.
"""
tunable_groups.assign(_TUNABLE_VALUES)
assert tunable_groups.is_updated()

tunable_groups.reset()
assert not tunable_groups.is_updated()

tunable_groups.assign(_TUNABLE_VALUES)
assert not tunable_groups.is_updated()


def test_tunable_group_update_kernel(tunable_groups: TunableGroups) -> None:
"""
Test that the is_updated flag is set only for the affected covariant group.
"""
tunable_groups.assign(_TUNABLE_VALUES)
assert tunable_groups.is_updated()
assert tunable_groups.is_updated(["kernel"])
assert not tunable_groups.is_updated(["boot", "provision"])


def test_tunable_group_update_boot(tunable_groups: TunableGroups) -> None:
"""
Test that the is_updated flag is set only for the affected covariant group.
"""
tunable_groups.assign(_TUNABLE_VALUES)
assert tunable_groups.is_updated()
assert not tunable_groups.is_updated(["boot"])

tunable_groups.reset()
tunable_groups["idle"] = "mwait"
assert tunable_groups.is_updated()
assert tunable_groups.is_updated(["boot"])
assert not tunable_groups.is_updated(["kernel"])
4 changes: 1 addition & 3 deletions mlos_bench/mlos_bench/tunables/covariant_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,6 @@ def __getitem__(self, tunable: Union[str, Tunable]) -> TunableValue:
return self.get_tunable(tunable).value

def __setitem__(self, tunable: Union[str, Tunable], tunable_value: Union[TunableValue, Tunable]) -> TunableValue:
self._is_updated = True
name: str = tunable.name if isinstance(tunable, Tunable) else tunable
value: TunableValue = tunable_value.value if isinstance(tunable_value, Tunable) else tunable_value
self._tunables[name].value = value
self._is_updated |= self.get_tunable(tunable).update(value)
return value
18 changes: 18 additions & 0 deletions mlos_bench/mlos_bench/tunables/tunable.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,24 @@ def value(self, value: TunableValue) -> TunableValue:
self._current_value = coerced_value
return self._current_value

def update(self, value: TunableValue) -> bool:
"""
Assign the value to the tunable. Return True if it is a new value, False otherwise.
Parameters
----------
value : Union[int, float, str]
Value to assign.
Returns
-------
is_updated : bool
True if the new value is different from the previous one, False otherwise.
"""
prev_value = self._current_value
self.value = value
return prev_value != self._current_value

def is_valid(self, value: TunableValue) -> bool:
"""
Check if the value can be assigned to the tunable.
Expand Down

0 comments on commit eb6205d

Please sign in to comment.