checkpoints: create final checkpoint commit after repro when needed (#4715)

* repo: make Repo.scm a cached attribute

* checkpoints: create final checkpoint after repro when needed

* use correct exception rev in _collect_and_commit

dvc/repo/__init__.py

@@ -121,8 +121,7 @@ class Repo:
             self.tree = LocalTree(self, {"url": self.root_dir}, **tree_kwargs)
 
         self.config = Config(self.dvc_dir, tree=self.tree)
-        no_scm = self.config["core"].get("no_scm", False)
-        self.scm = scm if scm else SCM(self.root_dir, no_scm=no_scm)
+        self._scm = scm
 
         # used by RepoTree to determine if it should traverse subrepos
         self.subrepos = subrepos
@@ -158,6 +157,13 @@ class Repo:
         self.plots = Plots(self)
         self.params = Params(self)
 
+    @cached_property
+    def scm(self):
+        from dvc.scm import SCM
+
+        no_scm = self.config["core"].get("no_scm", False)
+        return self._scm if self._scm else SCM(self.root_dir, no_scm=no_scm)
+
     @property
     def tree(self):
         return self._tree

dvc/repo/experiments/__init__.py

@@ -582,7 +582,7 @@ class Experiments:
                         result[rev] = {exp_rev: exp_hash}
                 else:
                     logger.exception(
-                        "Failed to reproduce experiment '%s'", rev
+                        "Failed to reproduce experiment '%s'", rev[:7]
                     )
                 executor.cleanup()
 
@@ -613,7 +613,8 @@ class Experiments:
                     result[rev] = {exp_rev: exp_hash}
 
             checkpoint_func = partial(_checkpoint_callback, rev, executor)
-            executor.reproduce(
+
+            exp_hash = executor.reproduce(
                 executor.dvc_dir,
                 cwd=executor.dvc.root_dir,
                 checkpoint=True,
@@ -621,14 +622,19 @@ class Experiments:
                 **executor.repro_kwargs,
             )
 
-            # TODO: determine whether or not we should create a final
-            # checkpoint commit after repro is killed, or if we should only do
-            # it on explicit make_checkpoint() signals.
+            # NOTE: GitPython Repo instances cannot be re-used after
+            # process has received SIGINT or SIGTERM, so we need this hack
+            # to re-instantiate git instances after checkpoint runs. See:
+            # https://github.com/gitpython-developers/GitPython/issues/427
+            del self.repo.scm
+            del self.scm
 
-        # NOTE: our cached GitPython Repo instance cannot be re-used if the
-        # checkpoint run was interrupted via SIGINT, so we need this hack
-        # to create a new git repo instance after checkpoint runs.
-        del self.scm
+            # Create final checkpoint commit if needed
+            exp_rev = self._collect_and_commit(
+                rev, executor, exp_hash, checkpoint=True
+            )
+            if exp_rev not in result[rev]:
+                result[rev] = {exp_rev: exp_hash}
 
         return result
 
@@ -652,13 +658,11 @@ class Experiments:
                 checkpoint_reset=executor.checkpoint_reset,
                 **kwargs,
             )
-        except UnchangedExperimentError:
+        except UnchangedExperimentError as exc:
             logger.debug(
-                "Experiment '%s' identical to baseline '%s'",
-                rev,
-                executor.baseline_rev,
+                "Experiment '%s' identical to '%s'", rev, exc.rev,
             )
-            exp_rev = executor.baseline_rev
+            exp_rev = exc.rev
         return exp_rev
 
     def _collect_input(self, executor: ExperimentExecutor):