search

theochem / procrustes

Python library for finding the optimal transformation(s) that makes two matrices as close as possible to each other.
https://procrustes.qcdevs.org/
GNU General Public License v3.0
109 stars 20 forks source link

Lapack driver #86

Closed Ali-Tehrani closed 3 years ago

Ali-Tehrani commented 3 years ago

This pull request is about adding the lapack driver option whenever singular value decomposition is used. See issue #78.

I added it to rotational, orthogonal, symmetric, permutation and generic. I also added tests to these. The doc string of "lapack_driver" for permutation and generic are different from the other modules.

For generic, I instead used NumPy's Morse Penrose inverse if "lapack_driver="gessd"" and SciPy's Morse Penrose inverse if "lapack_driver="gesvd"".

PaulWAyers commented 3 years ago

This seems like a sensible strategy to me. The alternative would be to use scipy in both cases, with pinv2 and pinv. I would have a slight preference for that as the scipy error tolerance is more sensible/refined.

Ali-Tehrani commented 3 years ago

After attempting your suggestion, I realized I made a mistake. pinv from SciPy uses least-squares approach to computing Morse-Penrose inverse and pinv2 uses singular value decomposition. I changed the lapack_driver argument to use_svd, and if it is True, then use pinv2 else use pinv. The default value is False, because it is more robust choice.