-
Notifications
You must be signed in to change notification settings - Fork 66
Description
The current API to compute/get a hessian in matscipy is the following function:
def get_hessian(self,
atoms: ase.Atoms,
format: str = 'sparse',
divide_by_masses: bool = False) -> Union[scipy.sparse.spmatrix, np.ndarray]:
...This has the following disatvantages:
- interoperability with ASE's property system is limited since there are function parameters. I've worked around that in
MatscipyCalculatorby defining different properties for different parameter combinations (e.g."dynamical_matrix"calls withdivide_by_masses=True. - Not every calculator returns a sparse matrix (e.g. Ewald), which means conversions must sometimes happen
- The sparse format should be
bsr_matrixfor all calculators: this makes it harder to change for specific calculators in case it's sub-optimal, and makes us less interoperable with other codes or future calculators that might use a different format, e.g. hierarchical matrices, or matrix-free formulations (other kspace implementations, fast-multipole, etc.) - There are two accepted formats:
"sparse"and"neighbor-list", which are not interoperable
To solve these issues, I propose a new API:
def get_hessian(self,
atoms: ase.Atoms) -> scipy.sparse.linalg.LinearOperator:
...The abstract class LinearOperator is designed to abstract away details of how matrix-vector (or matrix-matrix) products are done and are fully composable, i.e. P = A * B and S = A + B can be defined for A and B that have different representation without needing conversion. They are also accepted as parameters to the sparse solve/diagonalization routines in scipy. Most calculators in matscipy would just need to call scipy.sparse.linalg.aslinearoperator() on the sparse matrix that they assemble, which returns a concrete instance of MatrixLinearOperator, from which the matrix representation can be retrieved, if need be (e.g. for tests). This addresses points 2 & 3 above.
For point 1, the divide by masses operation should be done in a separate function. For point 4, after discussing with @griessej , we figured that the neighbor-list format is only really used in the pair and polydisperse calculators for the calculation of the Born constants. It then makes sense to move whatever code in MatscipyCalculator that uses that format to these calculators (and actually have the polydisperse calculator inherit from the pair calculator).
LMK what you think of this change. The main burden of making this change would probably be to change the tests of hessian values.