Skip to content

Commit

Permalink
Remove deprecated get method calls (#238)
Browse files Browse the repository at this point in the history
* Removal of deprecated functions [skip actions]

* Add force constant [skip actions]

* NEB refactoring

* Fix units for div mul

* Update changelog

* PR suggestions

---------

Co-authored-by: Shoubhik Maiti <17470159+shoubhikraj@users.noreply.github.com>
  • Loading branch information
t-young31 and shoubhikraj authored Mar 18, 2023
1 parent 5f355e7 commit 784b579
Show file tree
Hide file tree
Showing 36 changed files with 694 additions and 622 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ autode\.egg-info/
doc/_build/
**.DS_Store
**htmlcov/
.coverage
.coverage*
coverage.xml
tests/*.xyz
**/tmp.xyz
Expand All @@ -20,3 +20,4 @@ tests/*.xyz
*.inp
*.pyc
.autode_calculations
tests/initial_path/*
3 changes: 3 additions & 0 deletions autode/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from autode import neb
from autode import mol_graphs
from autode import hessians
from autode.neb import NEB, CINEB
from autode.reactions.reaction import Reaction
from autode.reactions.multistep import MultiStepReaction
from autode.transition_states.transition_state import TransitionState
Expand Down Expand Up @@ -61,6 +62,8 @@
"NCIComplex",
"Config",
"Calculation",
"NEB",
"CINEB",
"pes",
"neb",
"geom",
Expand Down
45 changes: 0 additions & 45 deletions autode/calculations/calculation.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,8 @@

from copy import deepcopy
from typing import Optional, List
from autode.utils import deprecated
from autode.atoms import Atoms
from autode.point_charges import PointCharge
from autode.log import logger
from autode.hessians import Hessian
from autode.values import PotentialEnergy, Gradient
from autode.calculations.types import CalculationType
from autode.calculations.executors import (
CalculationExecutor,
Expand Down Expand Up @@ -305,47 +301,6 @@ def _check_properties_exist(self) -> None:

return None

# ----------------- Deprecated functions ----------------------

@deprecated
def get_energy(self) -> PotentialEnergy:
self._executor.set_properties()
return self.molecule.energy

@deprecated
def optimisation_converged(self) -> bool:
self._executor.set_properties()
return self.optimiser.converged

@deprecated
def optimisation_nearly_converged(self) -> bool:
self._executor.set_properties()
tol = PotentialEnergy(0.1, units="kcal mol-1")
return (
not self.optimiser.converged
and self.optimiser.last_energy_change < tol
)

@deprecated
def get_final_atoms(self) -> Atoms:
self._executor.set_properties()
return self.molecule.atoms

@deprecated
def get_atomic_charges(self) -> List[float]:
self._executor.set_properties()
return self.molecule.partial_charges

@deprecated
def get_gradients(self) -> Gradient:
self._executor.set_properties()
return self.molecule.gradient

@deprecated
def get_hessian(self) -> Hessian:
self._executor.set_properties()
return self.molecule.hessian


def _are_opt(keywords) -> bool:
return isinstance(keywords, kws.OptKeywords)
Expand Down
8 changes: 8 additions & 0 deletions autode/constraints.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from collections.abc import MutableMapping
from typing import Optional, Dict, List
from copy import deepcopy

from autode.values import Distance
from autode.log import logger

Expand Down Expand Up @@ -122,6 +124,9 @@ def update(

return None

def copy(self) -> "Constraints":
return deepcopy(self)


class DistanceConstraints(MutableMapping):
def __init__(self, *args, **kwargs):
Expand Down Expand Up @@ -178,3 +183,6 @@ def __setitem__(self, key, value):
)

self._store[self._key_transform(key)] = Distance(value)

def copy(self) -> "DistanceConstraints":
return deepcopy(self)
19 changes: 8 additions & 11 deletions autode/neb/ci.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,26 @@
https://doi.org/10.1063/1.1329672
"""
import numpy as np
from scipy.optimize import OptimizeResult
from typing import Optional
from autode.neb.original import NEB, Images, Image
from autode.log import logger


class CImage(Image):
def __init__(self, image):
def __init__(self, image: Image):
"""
Construct a climbing image from a non-climbing one
-----------------------------------------------------------------------
Arguments:
image (autode.neb.Image):
"""
super().__init__(image.name, image.k)
super().__init__(species=image, name=image.name, k=image.k)
# Set all the current attributes from the regular image
self.__dict__.update(image.__dict__)

def get_force(self, im_l, im_r) -> np.ndarray:
def get_force(self, im_l: Image, im_r: Image) -> np.ndarray:
"""
Compute F_m
Expand All @@ -34,14 +35,13 @@ def get_force(self, im_l, im_r) -> np.ndarray:
hat_tau, x_l, x, x_r = self._tau_xl_x_xr(im_l, im_r)

# F_m = ∇V(x_m) + (2∇V(x_m).τ)τ
return -self.grad + 2.0 * np.dot(self.grad, hat_tau) * hat_tau
return -self.gradient + 2.0 * np.dot(self.gradient, hat_tau) * hat_tau


class CImages(Images):
def __init__(
self,
images: Images,
num: int = None,
wait_iterations: int = 4,
init_k: Optional[float] = None,
):
Expand All @@ -55,11 +55,10 @@ def __init__(
wait_iterations (int): Number of iterations to wait before turning
on the climbing image
init_k: Initial force constant
init_k: Initial force constant. Must be defined if the images are empty
"""
super().__init__(
num=num if num is not None else len(images),
init_k=init_k if init_k is not None else images[0].k,
init_k=init_k if init_k is not None else images.init_k,
)

self.wait_iteration = wait_iterations
Expand Down Expand Up @@ -102,9 +101,7 @@ def __init__(self, *args, **kwargs):

self.images = CImages(self.images)

def _minimise(
self, method, n_cores, etol, max_n=30
) -> "scipy.optimize.OptimizeResult":
def _minimise(self, method, n_cores, etol, max_n=30) -> OptimizeResult:
"""Minimise th energy of every image in the NEB"""
logger.info(f"Minimising to ∆E < {etol:.4f} Ha on all NEB coordinates")
result = super()._minimise(method, n_cores, etol, max_n)
Expand Down
27 changes: 17 additions & 10 deletions autode/neb/idpp.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import numpy as np

from scipy.spatial import distance_matrix
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from autode.neb.original import Images, Image


class IDPP:
Expand All @@ -17,7 +22,7 @@ class IDPP:
as suggested in the paper.
"""

def __init__(self, images: "autode.neb.original.Images"):
def __init__(self, images: "Images"):
"""Initialise a IDPP potential from a set of NEB images"""

if len(images) < 2:
Expand All @@ -29,7 +34,7 @@ def __init__(self, images: "autode.neb.original.Images"):

self._set_distance_matrices(images)

def __call__(self, image: "autode.neb.original.Image") -> float:
def __call__(self, image: "Image") -> float:
r"""
Value of the IDPP objective function for a single image defined by,
Expand All @@ -52,7 +57,7 @@ def __call__(self, image: "autode.neb.original.Image") -> float:

return 0.5 * np.sum(w * (r_k - r) ** 2)

def grad(self, image: "autode.neb.original.Image") -> np.ndarray:
def grad(self, image: "Image") -> np.ndarray:
r"""
Gradient of the potential with respect to displacement of
the Cartesian components: :math:`\nabla S = (dS/dx_0, dS/dy_0, dS/dz_0,
Expand All @@ -75,7 +80,7 @@ def grad(self, image: "autode.neb.original.Image") -> np.ndarray:
(np.ndarray): :math:`\nabla S`
"""

x = np.array(image.species.coordinates).flatten()
x = np.array(image.coordinates).flatten()
grad = np.zeros_like(x)

r = self._distance_matrix(image, unity_diagonal=True)
Expand Down Expand Up @@ -105,9 +110,9 @@ def grad(self, image: "autode.neb.original.Image") -> np.ndarray:
grad[1::3] = np.sum(a * delta[1::3, 1::3], axis=1) # y
grad[2::3] = np.sum(a * delta[2::3, 2::3], axis=1) # z

return grad
return grad.reshape((-1, 3))

def _set_distance_matrices(self, images) -> None:
def _set_distance_matrices(self, images: "Images") -> None:
"""
For each image determine the optimum distance matrix using
Expand All @@ -130,22 +135,24 @@ def _set_distance_matrices(self, images) -> None:
self._diagonal_distance_matrix_idxs = np.diag_indices_from(delta)
return None

def _req_distance_matrix(self, image):
def _req_distance_matrix(self, image: "Image"):
"""Required distance matrix for an image, with elements r_{ij}^k"""
return self._dists[image.name]

def _distance_matrix(self, image, unity_diagonal=False) -> np.ndarray:
def _distance_matrix(
self, image: "Image", unity_diagonal: bool = False
) -> np.ndarray:
"""Distance matrix for an image"""

x = image.species.coordinates
x = image.coordinates
r = distance_matrix(x, x)

if unity_diagonal:
r[self._diagonal_distance_matrix_idxs] = 1.0

return r

def _weight_matrix(self, image) -> np.ndarray:
def _weight_matrix(self, image: "Image") -> np.ndarray:
r"""
Weight matrix with elements
Expand Down
31 changes: 17 additions & 14 deletions autode/neb/neb.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
import autode.exceptions as ex

from autode.config import Config
from autode.log import logger
from autode.neb.ci import CINEB
from autode.transition_states.ts_guess import TSguess
from autode.utils import work_in

from typing import TYPE_CHECKING, Optional

if TYPE_CHECKING:
from autode.species.species import Species
from autode.wrappers.methods import Method


def get_ts_guess_neb(
reactant: "autode.species.Species",
product: "autode.species.Species",
method: "autode.wrappers.methods.Method",
reactant: "Species",
product: "Species",
method: "Method",
name: str = "neb",
n: int = 10,
):
) -> Optional[TSguess]:
"""
Get a transition state guess using a nudged elastic band calculation. The
geometry of the reactant is used as the fixed initial point and the final
Expand All @@ -37,25 +44,21 @@ def get_ts_guess_neb(
assert n is not None
logger.info("Generating a TS guess using a nudged elastic band")

neb = CINEB(
initial_species=reactant.copy(), final_species=product.copy(), num=n
)
neb = CINEB.from_end_points(reactant, product, num=n)

# Calculate and generate the TS guess, in a unique directory
try:

@work_in(name)
def calculate():
return neb.calculate(method=method, n_cores=Config.n_cores)
@work_in(name)
def calculate():
return neb.calculate(method=method, n_cores=Config.n_cores)

try:
calculate()

except ex.CalculationException:
logger.error("NEB failed - could not calculate")
return None

return TSguess(
atoms=neb.get_species_saddle_point().atoms,
atoms=neb.peak_species.atoms,
reactant=reactant,
product=product,
name=name,
Expand Down
Loading

0 comments on commit 784b579

Please sign in to comment.