address review feedback

RELEASE-NOTES.md

@@ -5,8 +5,8 @@
 ### New features
 - use [fastprogress](https://github.com/fastai/fastprogress) instead of tqdm [#3693](https://github.com/pymc-devs/pymc3/pull/3693)
 - `DEMetropolis` can now tune both `lambda` and `scaling` parameters, but by default neither of them are tuned. See [#3743](https://github.com/pymc-devs/pymc3/pull/3743) for more info.
-- `DEMetropolisZ`, an improved variant of `DEMetropolisZ` brings better parallelization and high efficiency with few chains with a slower initial convergence. This implementation is experimental. See [#3784](https://github.com/pymc-devs/pymc3/pull/3784) for more info.
-- Notebooks that give insight into `DEMetropolis`, `DEMetropolisZ` and the `DifferentialEquation` interface are now located in the [Tutorials/Deep Dive]() section.
+- `DEMetropolisZ`, an improved variant of `DEMetropolis` brings better parallelization and high efficiency with few chains with a slower initial convergence. This implementation is experimental. See [#3784](https://github.com/pymc-devs/pymc3/pull/3784) for more info.
+- Notebooks that give insight into `DEMetropolis`, `DEMetropolisZ` and the `DifferentialEquation` interface are now located in the [Tutorials/Deep Dive](https://docs.pymc.io/nb_tutorials/index.html) section.
 
 ### Maintenance
 - Remove `sample_ppc` and `sample_ppc_w` that were deprecated in 3.6.

pymc3/step_methods/metropolis.py

@@ -573,9 +573,10 @@ def __init__(self, vars=None, S=None, proposal_dist=None, lamb=None, scaling=0.0
 
         self.scaling = np.atleast_1d(scaling).astype('d')
         if lamb is None:
+            # default to the optimal lambda for normally distributed targets
             lamb = 2.38 / np.sqrt(2 * model.ndim)
         self.lamb = float(lamb)
-        if not tune in {None, 'scaling', 'lambda'}:
+        if tune not in {None, 'scaling', 'lambda'}:
             raise ValueError('The parameter "tune" must be one of {None, scaling, lambda}')
         self.tune = tune
         self.tune_interval = tune_interval
@@ -704,9 +705,10 @@ def __init__(self, vars=None, S=None, proposal_dist=None, lamb=None, scaling=0.0
 
         self.scaling = np.atleast_1d(scaling).astype('d')
         if lamb is None:
+            # default to the optimal lambda for normally distributed targets
             lamb = 2.38 / np.sqrt(2 * model.ndim)
         self.lamb = float(lamb)
-        if not tune in {None, 'scaling', 'lambda'}:
+        if tune not in {None, 'scaling', 'lambda'}:
             raise ValueError('The parameter "tune" must be one of {None, scaling, lambda}')
         self.tune = True
         self.tune_target = tune
@@ -732,6 +734,7 @@ def __init__(self, vars=None, S=None, proposal_dist=None, lamb=None, scaling=0.0
         super().__init__(vars, shared)
 
     def reset_tuning(self):
+        """Resets the tuned sampler parameters and history to their initial values."""
         # history can't be reset via the _untuned_settings dict because it's a list
         self._history = []
         for attr, initial_value in self._untuned_settings.items():
@@ -787,8 +790,9 @@ def astep(self, q0):
         return q_new, [stats]
 
     def stop_tuning(self):
-        # remove the first x% of the tuning phase so future steps are not
-        # informed by bad unconverged tuning iterations
+        """At the end of the tuning phase, this method removes the first x% of the history
+        so future proposals are not informed by unconverged tuning iterations.
+        """
         it = len(self._history)
         n_drop = int(self.tune_drop_fraction * it)
         self._history = self._history[n_drop:]

pymc3/tests/test_step.py

@@ -859,40 +859,38 @@ def test_tuning_reset(self):
         pass
 
     def test_tune_drop_fraction(self):
+        tune = 300
+        tune_drop_fraction = 0.85
+        draws = 200
         with Model() as pmodel:
             Normal('n', 0, 2, shape=(3,))
-            step = DEMetropolisZ(tune_drop_fraction=0.85)
+            step = DEMetropolisZ(tune_drop_fraction=tune_drop_fraction)
             trace = sample(
-                tune=300,
-                draws=200,
+                tune=tune,
+                draws=draws,
                 step=step,
                 cores=1,
                 chains=1,
                 discard_tuned_samples=False
             )
-            assert len(trace) == 500
-            assert len(step._history) == (300 - 300 * 0.85) + 200
+            assert len(trace) == tune + draws
+            assert len(step._history) == (tune - tune * tune_drop_fraction) + draws
         pass
 
-    def test_competence(self):
+    @pytest.mark.parametrize('variable,has_grad,outcome', [('n', True, 1),('n', False, 1),('b', True, 0),('b', False, 0)])
+    def test_competence(self, variable, has_grad, outcome):
         with Model() as pmodel:
-            n = Normal('n', 0, 2, shape=(3,))
-            b = Binomial('b', n=2, p=0.3)
-        assert DEMetropolisZ.competence(n, has_grad=True) == 1
-        assert DEMetropolisZ.competence(n, has_grad=False) == 1
-        assert DEMetropolisZ.competence(b, has_grad=True) == 0
-        assert DEMetropolisZ.competence(b, has_grad=False) == 0
+            Normal('n', 0, 2, shape=(3,))
+            Binomial('b', n=2, p=0.3)
+        assert DEMetropolisZ.competence(pmodel[variable], has_grad=has_grad) == outcome
         pass
 
-    def test_invalid_tune(self):
+    @pytest.mark.parametrize('tune_setting', ['foo', True, False])
+    def test_invalid_tune(self, tune_setting):
         with Model() as pmodel:
             Normal('n', 0, 2, shape=(3,))
             with pytest.raises(ValueError):
-                DEMetropolisZ(tune='foo')
-            with pytest.raises(ValueError):
-                DEMetropolisZ(tune=True)
-            with pytest.raises(ValueError):
-                DEMetropolisZ(tune=False)
+                DEMetropolisZ(tune=tune_setting)
         pass
 
     def test_custom_proposal_dist(self):