Repository: duartegroup/autodE
Pull request #271 (2023-04-13 19:41:51)

Raise exception if prune returns no conformers
Raises a more helpful exception in e.g #268 

### Questions

-  Is this behaviour intuitive? Certainly useful for debugging but are there circumstances where you'd want to call `prune` and allow an empty result?

codecov[bot]: ## [Codecov](https://codecov.io/gh/duartegroup/autodE/pull/271?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=duartegroup) Report
> Merging [#271](https://codecov.io/gh/duartegroup/autodE/pull/271?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=duartegroup) (16a11f1) into [v1.4.0](https://codecov.io/gh/duartegroup/autodE/commit/fe54c1971a2223c44e594807ce60a0386740ddc9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=duartegroup) (fe54c19) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##           v1.4.0     #271   +/-   ##
=======================================
  Coverage   96.81%   96.81%           
=======================================
  Files         197      197           
  Lines       21006    21023   +17     
=======================================
+ Hits        20336    20353   +17     
  Misses        670      670           
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `96.81% <100.00%> (+<0.01%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=duartegroup#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/duartegroup/autodE/pull/271?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=duartegroup) | Coverage Δ | |
|---|---|---|
| [autode/conformers/conformers.py](https://codecov.io/gh/duartegroup/autodE/pull/271?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=duartegroup#diff-YXV0b2RlL2NvbmZvcm1lcnMvY29uZm9ybWVycy5weQ==) | `100.00% <100.00%> (ø)` | |
| [tests/test\_conformers.py](https://codecov.io/gh/duartegroup/autodE/pull/271?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=duartegroup#diff-dGVzdHMvdGVzdF9jb25mb3JtZXJzLnB5) | `99.53% <100.00%> (+0.02%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=duartegroup). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=duartegroup)


Patch:
diff --git a/autode/conformers/conformers.py b/autode/conformers/conformers.py
index 7b668e6..9992c08 100644
--- a/autode/conformers/conformers.py
+++ b/autode/conformers/conformers.py
@@ -1,6 +1,7 @@
 import numpy as np
 from typing import Optional, Union
 from rdkit import Chem
+
 from autode.values import Distance, Energy
 from autode.atoms import Atom, Atoms
 from autode.config import Config
@@ -8,6 +9,7 @@ from autode.mol_graphs import make_graph, is_isomorphic
 from autode.geom import calc_heavy_atom_rmsd
 from autode.log import logger
 from autode.utils import ProcessPool
+from autode.exceptions import NoConformers
 
 
 def _calc_conformer(conformer, calc_type, method, keywords, n_cores=1):
@@ -61,9 +63,7 @@ class Conformers(list):
         """
 
         if remove_no_energy:
-            for idx in reversed(range(len(self))):  # Enumerate backwards
-                if self[idx].energy is None:
-                    del self[idx]
+            self.remove_no_energy()
 
         self.prune_on_energy(e_tol=e_tol, n_sigma=n_sigma)
         self.prune_on_rmsd(rmsd_tol=rmsd_tol)
@@ -234,6 +234,21 @@ class Conformers(list):
         logger.info(f"Pruned on connectivity {n_prev_confs} -> {len(self)}")
         return None
 
+    def remove_no_energy(self) -> None:
+        """Remove all conformers from this list that do not have an energy"""
+        n_conformers_before_remove = len(self)
+
+        for idx in reversed(range(len(self))):  # Enumerate backwards
+            if self[idx].energy is None:
+                del self[idx]
+
+        n_conformers = len(self)
+        if n_conformers == 0 and n_conformers != n_conformers_before_remove:
+            raise NoConformers(
+                f"Removed all the conformers "
+                f"{n_conformers_before_remove} -> 0"
+            )
+
     def _parallel_calc(self, calc_type, method, keywords):
         """
         Run a set of calculations (single point energy evaluations or geometry
diff --git a/autode/neb/original.py b/autode/neb/original.py
index 88d15f5..2b9b77a 100644
--- a/autode/neb/original.py
+++ b/autode/neb/original.py
@@ -517,6 +517,9 @@ class NEB:
                 "Cannot construct a NEB from species with different atoms"
             )
 
+        if num < 2:
+            raise ValueError("Cannot create a NEB with fewer than 2 images")
+
         neb = cls.from_list(
             species_list=cls._interpolated_species(initial, final, n=num),
             init_k=init_k,
@@ -613,6 +616,9 @@ class NEB:
         """Generate simple interpolated coordinates for these set of images
         in Cartesian coordinates"""
 
+        if n == 2:
+            return [initial.copy(), final.copy()]
+
         intermediate_species = []
 
         # Interpolate images between the starting point i=0 and end point i=n-1
diff --git a/doc/changelog.rst b/doc/changelog.rst
index 95cfc6b..99f5547 100644
--- a/doc/changelog.rst
+++ b/doc/changelog.rst
@@ -19,7 +19,7 @@ Usability improvements/Changes
 - The electronic temperature and the version of parameterisation for xTB calculations are made configurable
 - A NEB :code:`Image` now derives from a :code:`Species` superclass
 - Modifies NEB :code:`Image` constructor to be formed from an image
-- Defines named constructors (:code:`from_endpoints(...)`, :code:`from_list(...)`) for :code:`NEB`
+- Defines named constructors (:code:`from_end_points(...)`, :code:`from_list(...)`) for :code:`NEB`
 - Removes :code:`NEB().contains_peak()` in favour of :code:`NEB().images.contains_peak`
 - Modifies the :code:`CImages` constructor to ensure it's constructed from an :code:`Images` instance
 - Removes :code:`NEB.get_species_saddle_point()` in favour of :code:`NEB.peak_species`


Test patch:
diff --git a/tests/test_conformers.py b/tests/test_conformers.py
index dd901aa..75f944f 100644
--- a/tests/test_conformers.py
+++ b/tests/test_conformers.py
@@ -2,6 +2,7 @@ from autode.atoms import Atom, Atoms
 from autode.species import Molecule, NCIComplex
 from autode.conformers import Conformer, Conformers
 from autode.conformers.conformers import _calc_conformer
+from autode.exceptions import NoConformers
 from autode.wrappers.ORCA import ORCA
 from autode.wrappers.XTB import XTB
 from autode.config import Config
@@ -177,11 +178,14 @@ def test_confs_energy_pruning3():
 
 def test_confs_no_energy_pruning():
     # Check that if energies are unassigned then conformers are removed
+    conf0 = Conformer(atoms=[Atom("H")])
+    conf1 = conf0.copy()
+    conf1.energy = -0.5
 
-    confs = Conformers([Conformer(atoms=[Atom("H")])])
+    confs = Conformers([conf0, conf1])
     confs.prune(remove_no_energy=True)
 
-    assert len(confs) == 0
+    assert len(confs) == 1
 
 
 def test_confs_rmsd_pruning1():
@@ -382,3 +386,21 @@ def test_conformer_coordinate_setting_with_different_atomic_attr():
     # But cannot set atoms with a different label (e.g. atomic symbol)
     with pytest.raises(ValueError):
         conf.atoms = Atoms([Atom("H")])
+
+
+def test_pruning_conformers_without_energy_raises():
+
+    conformers = Conformers(
+        [Conformer(atoms=[Atom("H")]), Conformer(atoms=[Atom("H")])]
+    )
+
+    assert sum(c.energy is None for c in conformers) == 2
+
+    with pytest.raises(NoConformers):
+        conformers.prune(remove_no_energy=True)
+
+
+def test_pruning_no_energy_with_no_conformers_is_possible():
+
+    conformers = Conformers()
+    conformers.remove_no_energy()
diff --git a/tests/test_neb.py b/tests/test_neb.py
index e674091..ccfc8a0 100644
--- a/tests/test_neb.py
+++ b/tests/test_neb.py
@@ -421,3 +421,11 @@ def test_constructing_neb_from_endpoints_with_different_atoms_raises():
         _ = NEB.from_end_points(
             Molecule(smiles="O"), Molecule(smiles="C"), num=4
         )
+
+
+def test_neb_from_endpoints_requires_at_least_2_images():
+
+    with pytest.raises(Exception):
+        _ = NEB.from_end_points(
+            Molecule(smiles=r"C\C=C\C"), Molecule(smiles=r"C\C=C/C"), num=1
+        )