Repository: automl/SMAC3

Issue:
Path to scenario folder to restore state
Hi,

I was running the example on restoring the state for the branin function and I run into this error

```
INFO:smac.scenario.scenario.Scenario:Output to restored
Output directory= restore_me/run_1
['traj_old.csv', 'scenario.txt', 'runhistory.json', 'param_config_space.pcs', 'stats.json', 'traj_aclib2.json']
Traceback (most recent call last):
  File "restore_state.py", line 77, in <module>
    main()
  File "restore_state.py", line 64, in main
    shutil.copy(traj_path, new_traj_path)
  File "/usr/lib/python3.5/shutil.py", line 235, in copy
    copyfile(src, dst, follow_symlinks=follow_symlinks)
  File "/usr/lib/python3.5/shutil.py", line 115, in copyfile

    with open(dst, 'wb') as fdst:
FileNotFoundError: [Errno 2] No such file or directory: 'restored/traj_aclib2.json'
```

I figure the issue comes from this line:
https://github.com/automl/SMAC3/blob/56a5af9e3666ae54f7c126b62ae44852af65842d/examples/branin/restore_state.py#L46

The output scenario folder should redefined as `output_dir = os.path.join(original_scenario.output_dir, 'run_1')`, right?


Discussion:

Patch:
diff --git a/examples/branin/restore_state.py b/examples/branin/restore_state.py
index 12cb6b51d..55cce7426 100644
--- a/examples/branin/restore_state.py
+++ b/examples/branin/restore_state.py
@@ -23,12 +23,15 @@ def main():
         'deterministic' : True,
         'output_dir' : 'restore_me'}
     original_scenario = Scenario(orig_scen_dict)
-    smac = SMAC(scenario=original_scenario)
+    smac = SMAC(scenario=original_scenario,
+                run_id=1)
     smac.optimize()
 
     print("\n########## BUDGET EXHAUSTED! Restoring optimization: ##########\n")
 
-    # Now the output is in the folder 'restore_me'
+    # Now the output is in the folder 'restore_me/run_1' (or whatever run_id has
+    # been passed to the SMAC-object above)
+    old_output_dir = os.path.join(original_scenario.output_dir, 'run_1')
     #
     # We could simply modify the scenario-object, stored in
     # 'smac.solver.scenario' and start optimization again:
@@ -43,22 +46,18 @@ def main():
                       'output_dir' : 'restored'})
 
     # We load the runhistory, ...
-    rh_path = os.path.join(original_scenario.output_dir, "runhistory.json")
+    rh_path = os.path.join(old_output_dir, "runhistory.json")
     runhistory = RunHistory(aggregate_func=None)
     runhistory.load_json(rh_path, new_scenario.cs)
     # ... stats, ...
-    stats_path = os.path.join(original_scenario.output_dir, "stats.json")
+    stats_path = os.path.join(old_output_dir, "stats.json")
     stats = Stats(new_scenario)
     stats.load(stats_path)
     # ... and trajectory.
-    traj_path = os.path.join(original_scenario.output_dir, "traj_aclib2.json")
+    traj_path = os.path.join(old_output_dir,  "traj_aclib2.json")
     trajectory = TrajLogger.read_traj_aclib_format(
         fn=traj_path, cs=new_scenario.cs)
     incumbent = trajectory[-1]["incumbent"]
-    # Because we changed the output_dir, we might want to copy the old
-    # trajectory-file (runhistory and stats will be complete)
-    new_traj_path = os.path.join(new_scenario.output_dir, "traj_aclib2.json")
-    shutil.copy(traj_path, new_traj_path)
 
     # Now we can initialize SMAC with the recovered objects and restore the
     # state where we left off. By providing stats and a restore_incumbent, SMAC
@@ -66,7 +65,14 @@ def main():
     smac = SMAC(scenario=new_scenario,
                 runhistory=runhistory,
                 stats=stats,
-                restore_incumbent=incumbent)
+                restore_incumbent=incumbent,
+                run_id=1)
+    # Because we changed the output_dir, we might want to copy the old
+    # trajectory-file (runhistory and stats will be complete, but trajectory is
+    # written sequentially)
+    new_traj_path = os.path.join(new_scenario.output_dir, "run_1",  "traj_aclib2.json")
+    shutil.copy(traj_path, new_traj_path)
+
     smac.optimize()
 
 if "__main__" == __name__:
diff --git a/smac/facade/smac_facade.py b/smac/facade/smac_facade.py
index 3368bac94..5b315dec3 100644
--- a/smac/facade/smac_facade.py
+++ b/smac/facade/smac_facade.py
@@ -128,15 +128,26 @@ class SMAC(object):
         aggregate_func = average_cost
 
         self.scenario = scenario
-        self.output_dir = create_output_directory(scenario, run_id)
+        self.output_dir = ""
+        if not restore_incumbent:
+            self.output_dir = create_output_directory(scenario, run_id)
+        elif scenario.output_dir is not None:
+            # output-directory is created in CLI when restoring from a
+            # folder. calling the function again in the facade results in two
+            # folders being created: run_X and run_X.OLD. if we are
+            # restoring, the output-folder exists already and we omit creating it,
+            # but set the self-output_dir to the dir.
+            # necessary because we want to write traj to new output-dir in CLI.
+            self.output_dir = os.path.join(scenario.output_dir,
+                                           "run_%d" % (run_id))
         if (
             scenario.deterministic is True
             and getattr(scenario, 'tuner_timeout', None) is None
             and scenario.run_obj == 'quality'
         ):
             self.logger.info('Optimizing a deterministic scenario for '
-                             'qualitiy without a tuner timeout - will make '
-                             'SMAC deterministi!')
+                             'quality without a tuner timeout - will make '
+                             'SMAC deterministic!')
             scenario.intensification_percentage = 1e-10
         scenario.write()
 
@@ -175,7 +186,7 @@ class SMAC(object):
         # initial acquisition function
         if acquisition_function is None:
             acquisition_function = EI(model=model)
-            
+
         # inject model if necessary
         if acquisition_function.model is None:
             acquisition_function.model = model
diff --git a/smac/smac_cli.py b/smac/smac_cli.py
index b2382c0c7..4bab14f49 100644
--- a/smac/smac_cli.py
+++ b/smac/smac_cli.py
@@ -64,21 +64,19 @@ class SMACCLI(object):
         stats = None
         incumbent = None
 
-        # Restore state (needs to be before scenario-creation!)
-        if args_.restore_state:
-            root_logger.debug("Restoring state from %s...", args_.restore_state)
-            rh, stats, traj_list_aclib, traj_list_old = self.restore_state_before_scen(args_)
-
         # Create scenario-object
         scen = Scenario(args_.scenario_file, misc_args)
 
-        # Restore state (continued, needs to be after scenario-creation!)
+        # Restore state
         if args_.restore_state:
+            root_logger.debug("Restoring state from %s...", args_.restore_state)
+            rh, stats, traj_list_aclib, traj_list_old = self.restore_state(scen, args_)
+
             scen.output_dir_for_this_run = create_output_directory(
                 scen, args_.seed, root_logger,
             )
             scen.write()
-            stats, incumbent = self.restore_state_after_scen(scen, stats,
+            incumbent = self.restore_state_after_output_dir(scen, stats,
                                            traj_list_aclib, traj_list_old)
 
         if args_.warmstart_runhistory:
@@ -129,12 +127,9 @@ class SMACCLI(object):
         except (TAEAbortException, FirstRunCrashedException) as err:
             self.logger.error(err)
 
-    def restore_state_before_scen(self, args_):
+    def restore_state(self, scen, args_):
         """Read in files for state-restoration: runhistory, stats, trajectory.
         """
-        # Construct dummy-scenario for object-creation (mainly cs is needed)
-        tmp_scen = InputReader().read_scenario_file(args_.scenario_file)
-        tmp_scen = Scenario(tmp_scen, cmd_args={'output_dir':''})
         # Check for folder and files
         rh_path = os.path.join(args_.restore_state, "runhistory.json")
         stats_path = os.path.join(args_.restore_state, "stats.json")
@@ -145,9 +140,9 @@ class SMACCLI(object):
            raise FileNotFoundError("Could not find folder from which to restore.")
         # Load runhistory and stats
         rh = RunHistory(aggregate_func=None)
-        rh.load_json(rh_path, tmp_scen.cs)
+        rh.load_json(rh_path, scen.cs)
         self.logger.debug("Restored runhistory from %s", rh_path)
-        stats = Stats(tmp_scen)  # Need to inject actual scenario later for output_dir!
+        stats = Stats(scen)
         stats.load(stats_path)
         self.logger.debug("Restored stats from %s", stats_path)
         with open(traj_path_aclib, 'r') as traj_fn:
@@ -156,13 +151,11 @@ class SMACCLI(object):
             traj_list_old = traj_fn.readlines()
         return rh, stats, traj_list_aclib, traj_list_old
 
-    def restore_state_after_scen(self, scen, stats, traj_list_aclib,
-                                 traj_list_old):
-        """Finish processing files for state-restoration. The actual scenario
-        needs to be injected into stats, as well as the trajectory dealt with
-        (it is read in, but needs to be written to new output-folder after
-        scenario is constructed."""
-        stats.scenario = scen  # inject actual scen for output_dir
+    def restore_state_after_output_dir(self, scen, stats, traj_list_aclib,
+                                       traj_list_old):
+        """Finish processing files for state-restoration. Trajectory
+        is read in, but needs to be written to new output-folder. Therefore, the
+        output-dir is created. This needs to be considered in the SMAC-facade."""
         # write trajectory-list
         traj_path_aclib = os.path.join(scen.output_dir, "traj_aclib2.json")
         traj_path_old = os.path.join(scen.output_dir, "traj_old.csv")
@@ -171,8 +164,9 @@ class SMACCLI(object):
         with open(traj_path_old, 'w') as traj_fn:
             traj_fn.writelines(traj_list_old)
         # read trajectory to retrieve incumbent
+        # TODO replace this with simple traj_path_aclib?
         trajectory = TrajLogger.read_traj_aclib_format(fn=traj_path_aclib, cs=scen.cs)
         incumbent = trajectory[-1]["incumbent"]
         self.logger.debug("Restored incumbent %s from %s", incumbent,
                           traj_path_aclib)
-        return stats, incumbent
+        return incumbent


Test patch:
diff --git a/test/test_cli/test_restore_state.py b/test/test_cli/test_restore_state.py
index 8bcc3f066..1459b5fa9 100644
--- a/test/test_cli/test_restore_state.py
+++ b/test/test_cli/test_restore_state.py
@@ -40,6 +40,7 @@ class TestSMACCLI(unittest.TestCase):
         for output_dir in self.output_dirs:
             if output_dir:
                 shutil.rmtree(output_dir, ignore_errors=True)
+                #pass
         os.chdir(self.current_dir)
 
     @attr('slow')
@@ -100,7 +101,12 @@ class TestSMACCLI(unittest.TestCase):
         # Increase limit and run for 10 (so 5 more) by using restore_state
         testargs = ["python", "scripts/smac", "--restore_state",
                     self.output_one, "--scenario_file",
-                    self.scenario_one, "--verbose", "DEBUG"]
+                    self.scenario_two, "--verbose", "DEBUG"]
         with mock.patch.object(sys, 'argv', testargs):
             self.smaccli.main_cli()
+        self.assertTrue(os.path.exists(self.output_one))
+        self.assertFalse(os.path.exists(self.output_one + '.OLD'))
+        self.assertTrue(os.path.exists(self.output_two))
+        self.assertFalse(os.path.exists(self.output_two + '.OLD'))
+