[FIX] prioritize cli options over yaml (#5495)

* prioritize cli over yaml in checkpointSettings

* prioritize resume if both set in one place

* fixed test_commandline_args

* addressing comments:renaming, commenting

* more tests and referenced in change_log

com.unity.ml-agents/CHANGELOG.md

@@ -19,7 +19,7 @@ and this project adheres to
 - Fixed a bug in multi-agent cooperative training where agents might not receive all of the states of
 terminated teammates. (#5441)
 - Fixed wrong attribute name in argparser for torch device option (#5433)(#5467)
-
+- Fixed conflicting CLI and yaml options regarding resume & initialize_from (#5495)
 ## [2.1.0-exp.1] - 2021-06-09
 ### Minor Changes
 #### com.unity.ml-agents / com.unity.ml-agents.extensions (C#)

ml-agents/mlagents/trainers/settings.py

@@ -733,7 +733,7 @@ def __missing__(self, key: Any) -> "TrainerSettings":
                     f"Please add an entry in the configuration file for {key}, or set default_settings."
                 )
             else:
-                logger.warn(
+                logger.warning(
                     f"Behavior name {key} does not match any behaviors specified "
                     f"in the trainer configuration file. A default configuration will be used."
                 )
@@ -769,6 +769,32 @@ def maybe_init_path(self) -> Optional[str]:
     def run_logs_dir(self) -> str:
         return os.path.join(self.write_path, "run_logs")
 
+    def prioritize_resume_init(self) -> None:
+        """Prioritize explicit command line resume/init over conflicting yaml options.
+        if both resume/init are set at one place use resume"""
+        _non_default_args = DetectDefault.non_default_args
+        if "resume" in _non_default_args:
+            if self.initialize_from is not None:
+                logger.warning(
+                    f"Both 'resume' and 'initialize_from={self.initialize_from}' are set!"
+                    f" Current run will be resumed ignoring initialization."
+                )
+                self.initialize_from = parser.get_default("initialize_from")
+        elif "initialize_from" in _non_default_args:
+            if self.resume:
+                logger.warning(
+                    f"Both 'resume' and 'initialize_from={self.initialize_from}' are set!"
+                    f" {self.run_id} is initialized_from {self.initialize_from} and resume will be ignored."
+                )
+                self.resume = parser.get_default("resume")
+        elif self.resume and self.initialize_from is not None:
+            # no cli args but both are set in yaml file
+            logger.warning(
+                f"Both 'resume' and 'initialize_from={self.initialize_from}' are set in yaml file!"
+                f" Current run will be resumed ignoring initialization."
+            )
+            self.initialize_from = parser.get_default("initialize_from")
+
 
 @attr.s(auto_attribs=True)
 class EnvironmentSettings:
@@ -888,6 +914,7 @@ def from_argparse(args: argparse.Namespace) -> "RunOptions":
                     configured_dict[key] = val
 
         final_runoptions = RunOptions.from_dict(configured_dict)
+        final_runoptions.checkpoint_settings.prioritize_resume_init()
         # 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

ml-agents/mlagents/trainers/tests/test_learn.py

@@ -133,7 +133,6 @@ def test_commandline_args(mock_file):
     full_args = [
         "mytrainerpath",
         "--env=./myenvfile",
-        "--resume",
         "--inference",
         "--run-id=myawesomerun",
         "--seed=7890",
@@ -156,6 +155,12 @@ def test_commandline_args(mock_file):
     assert opt.engine_settings.no_graphics is True
     assert opt.debug is True
     assert opt.checkpoint_settings.inference is True
+    assert opt.checkpoint_settings.resume is False
+
+    # ignore init if resume is set
+    full_args.append("--resume")
+    opt = parse_command_line(full_args)
+    assert opt.checkpoint_settings.initialize_from is None  # ignore init if resume set
     assert opt.checkpoint_settings.resume is True
 
 

ml-agents/mlagents/trainers/tests/test_settings.py

@@ -481,6 +481,10 @@ def test_exportable_settings(use_defaults):
     # Check that the two exports are the same
     assert dict_export == second_export
 
+    # check if cehckpoint_settings priorotizes resume over initialize from
+    run_options2.checkpoint_settings.prioritize_resume_init()
+    assert run_options2.checkpoint_settings.initialize_from is None
+
 
 def test_environment_settings():
     # default args