Skip to content

[debug] Require all behavior names to have a matching YAML entry #5210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions com.unity.ml-agents/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ sizes and will need to be retrained. (#5181)
depend on the previous behavior, you can explicitly set the Agent's `InferenceDevice` to `InferenceDevice.CPU`. (#5175)

#### ml-agents / ml-agents-envs / gym-unity (Python)
- When using a configuration YAML, it is required to define all behaviors found in a Unity
executable in the trainer configuration YAML, or specify `default_settings`. (#5210)

### Bug Fixes
#### com.unity.ml-agents / com.unity.ml-agents.extensions (C#)
Expand Down
29 changes: 26 additions & 3 deletions ml-agents/mlagents/trainers/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -718,12 +718,26 @@ def __init__(self, *args):
super().__init__(*args)
else:
super().__init__(TrainerSettings, *args)
self._config_specified = True

def set_config_specified(self, require_config_specified: bool) -> None:
self._config_specified = require_config_specified

def __missing__(self, key: Any) -> "TrainerSettings":
if TrainerSettings.default_override is not None:
return copy.deepcopy(TrainerSettings.default_override)
self[key] = copy.deepcopy(TrainerSettings.default_override)
elif self._config_specified:
raise TrainerConfigError(
f"The behavior name {key} has not been specified in the trainer configuration. "
f"Please add an entry in the configuration file for {key}."
)
else:
return TrainerSettings()
warnings.warn(
f"Behavior name {key} does not match any behaviors specified "
f"in the trainer configuration file. A default configuration will be used."
)
self[key] = TrainerSettings()
return self[key]


# COMMAND LINE #########################################################################
Expand Down Expand Up @@ -800,7 +814,8 @@ class RunOptions(ExportableSettings):
# These are options that are relevant to the run itself, and not the engine or environment.
# They will be left here.
debug: bool = parser.get_default("debug")
# Strict conversion

# Convert to settings while making sure all fields are valid
cattr.register_structure_hook(EnvironmentSettings, strict_to_cls)
cattr.register_structure_hook(EngineSettings, strict_to_cls)
cattr.register_structure_hook(CheckpointSettings, strict_to_cls)
Expand Down Expand Up @@ -839,8 +854,12 @@ def from_argparse(args: argparse.Namespace) -> "RunOptions":
"engine_settings": {},
"torch_settings": {},
}
_require_all_behaviors = True
if config_path is not None:
configured_dict.update(load_config(config_path))
else:
# If we're not loading from a file, we don't require all behavior names to be specified.
_require_all_behaviors = False

# Use the YAML file values for all values not specified in the CLI.
for key in configured_dict.keys():
Expand Down Expand Up @@ -868,6 +887,10 @@ def from_argparse(args: argparse.Namespace) -> "RunOptions":
configured_dict[key] = val

final_runoptions = RunOptions.from_dict(configured_dict)
# Need check to bypass type checking but keep structure on dict working
if isinstance(final_runoptions.behaviors, TrainerSettings.DefaultTrainerDict):
# configure whether or not we should require all behavior names to be found in the config YAML
final_runoptions.behaviors.set_config_specified(_require_all_behaviors)
return final_runoptions

@staticmethod
Expand Down
25 changes: 24 additions & 1 deletion ml-agents/mlagents/trainers/tests/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ def test_default_settings():
default_settings_cls = cattr.structure(default_settings, TrainerSettings)
check_if_different(default_settings_cls, run_options.behaviors["test2"])

# Check that an existing beehavior overrides the defaults in specified fields
# Check that an existing behavior overrides the defaults in specified fields
test1_settings = run_options.behaviors["test1"]
assert test1_settings.max_steps == 2
assert test1_settings.network_settings.hidden_units == 2000
Expand All @@ -519,6 +519,29 @@ def test_default_settings():
check_if_different(test1_settings, default_settings_cls)


def test_strict():
# Create normal non-strict RunOptions
behaviors = {"test1": {"max_steps": 2, "network_settings": {"hidden_units": 2000}}}
run_options_dict = {"behaviors": behaviors}
ro = RunOptions.from_dict(run_options_dict)
# Test that we can grab an entry that is not in the dict.
assert isinstance(ro.behaviors["test2"], TrainerSettings)

# Create strict RunOptions with no defualt_settings
run_options_dict = {"behaviors": behaviors, "strict": True}
ro = RunOptions.from_dict(run_options_dict)
with pytest.raises(TrainerConfigError):
# Variable must be accessed otherwise Python won't query the dict
print(ro.behaviors["test2"])

# Create strict RunOptions with default settings
default_settings = {"max_steps": 1, "network_settings": {"num_layers": 1000}}
run_options_dict = {"default_settings": default_settings, "behaviors": behaviors}
ro = RunOptions.from_dict(run_options_dict)
# Test that we can grab an entry that is not in the dict.
assert isinstance(ro.behaviors["test2"], TrainerSettings)


def test_pickle():
# Make sure RunOptions is pickle-able.
run_options = RunOptions()
Expand Down
5 changes: 0 additions & 5 deletions ml-agents/mlagents/trainers/trainer/trainer_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ def __init__(
self.ghost_controller = GhostController()

def generate(self, behavior_name: str) -> Trainer:
if behavior_name not in self.trainer_config.keys():
logger.warning(
f"Behavior name {behavior_name} does not match any behaviors specified"
f"in the trainer configuration file: {sorted(self.trainer_config.keys())}"
)
trainer_settings = self.trainer_config[behavior_name]
return TrainerFactory._initialize_trainer(
trainer_settings,
Expand Down