From 2ee12e53079d765ab1f145b50d6f17395ebb8166 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Tue, 15 Oct 2024 16:02:49 +1100 Subject: [PATCH 01/30] start of docs --- package/MDAnalysis/lib/distances.py | 30 ++++++++++++------- .../MDAnalysisTests/lib/test_distances.py | 24 ++++++++------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index 062b212ea30..600bc6cfc46 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -61,14 +61,22 @@ you wish to use is covered by distopia. For more information see the `distopia documentation`_. -.. Table:: Functions available using the `distopia`_ backend. - :align: center - - +-------------------------------------+-----------------------------------+ - | Functions | Notes | - +=====================================+===================================+ - | MDAnalysis.lib.distances.calc_bonds | Doesn't support triclinic boxes | - +-------------------------------------+-----------------------------------+ +.. table:: Functions available using the `distopia`_ backend. + :align: center + + +-----------------------------------------------+ + | Functions | + +===============================================+ + | MDAnalysis.lib.distances.calc_bonds | + +-----------------------------------------------+ + | MDAnalysis.lib.distances.calc_angles | + +-----------------------------------------------+ + | MDAnalysis.lib.distances.calc_dihedrals | + +-----------------------------------------------+ + | MDAnalysis.lib.distances.distance_array | + +-----------------------------------------------+ + | MDAnalysis.lib.distances.self_distance_array | + +-----------------------------------------------+ If `distopia`_ is installed, the functions in this table will accept the key 'distopia' for the `backend` keyword argument. If the distopia backend is @@ -83,7 +91,7 @@ .. Note:: Due to the use of Instruction Set Architecture (`ISA`_) specific SIMD - intrinsics in distopia via `VCL2`_, the precision of your results may + intrinsics in distopia via `HWY`_, the precision of your results may depend on the ISA available on your machine. However, in all tested cases distopia satisfied the accuracy thresholds used to the functions in this module. Please document any issues you encounter with distopia's accuracy @@ -92,7 +100,7 @@ .. _distopia: https://github.com/MDAnalysis/distopia .. _distopia documentation: https://www.mdanalysis.org/distopia .. _ISA: https://en.wikipedia.org/wiki/Instruction_set_architecture -.. _VCL2: https://github.com/vectorclass/version2 +.. _HWY: https://github.com/google/highway .. _relevant distopia issue: https://github.com/MDAnalysis/mdanalysis/issues/3915 .. versionadded:: 0.13.0 @@ -101,6 +109,8 @@ :class:`~MDAnalysis.core.groups.AtomGroup` or an :class:`np.ndarray` .. versionchanged:: 2.5.0 Interface to the `distopia`_ package added. +.. versionchanged:: 2.7.0 + Distopia support greatly expanded. Functions --------- diff --git a/testsuite/MDAnalysisTests/lib/test_distances.py b/testsuite/MDAnalysisTests/lib/test_distances.py index 3fe4b2852b8..2ef53bab427 100644 --- a/testsuite/MDAnalysisTests/lib/test_distances.py +++ b/testsuite/MDAnalysisTests/lib/test_distances.py @@ -33,6 +33,16 @@ from MDAnalysis.tests.datafiles import PSF, DCD, TRIC +def distopia_conditional_backend(): + # functions that allow distopia acceleration need to be tested with + # distopia backend argument but distopia is an optional dep. + if HAS_DISTOPIA: + return ["serial", "openmp", "distopia"] + else: + return ["serial", "openmp"] + + + class TestCheckResultArray(object): ref = np.zeros(1, dtype=np.float64) @@ -309,7 +319,7 @@ def ref_system_universe(ref_system): u.select_atoms("index 1 to 3")) -@pytest.mark.parametrize('backend', ['serial', 'openmp']) +@pytest.mark.parametrize("backend", distopia_conditional_backend()) class TestDistanceArray(object): @staticmethod def _dist(x, ref): @@ -826,14 +836,6 @@ def convert_position_dtype_if_ndarray(a, b, c, d, dtype): conv_dtype_if_ndarr(d, dtype)) -def distopia_conditional_backend(): - # functions that allow distopia acceleration need to be tested with - # distopia backend argument but distopia is an optional dep. - if HAS_DISTOPIA: - return ["serial", "openmp", "distopia"] - else: - return ["serial", "openmp"] - class TestCythonFunctions(object): # Unit tests for calc_bonds calc_angles and calc_dihedrals in lib.distances @@ -1138,7 +1140,7 @@ def test_numpy_compliance_bonds(self, positions, backend): err_msg="Cython bonds didn't match numpy calculations", ) - @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_numpy_compliance_angles(self, positions, backend): a, b, c, d = positions # Checks that the cython functions give identical results to the numpy versions @@ -1154,7 +1156,7 @@ def test_numpy_compliance_angles(self, positions, backend): err_msg="Cython angles didn't match numpy calcuations", ) - @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_numpy_compliance_dihedrals(self, positions, backend): a, b, c, d = positions # Checks that the cython functions give identical results to the numpy versions From 9bf0cf757577d09303e4592109e6bec09ef42d63 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Tue, 15 Oct 2024 16:09:40 +1100 Subject: [PATCH 02/30] improve distopia stup --- package/MDAnalysis/lib/_distopia.py | 79 +++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 10 deletions(-) diff --git a/package/MDAnalysis/lib/_distopia.py b/package/MDAnalysis/lib/_distopia.py index 7170cf2a556..4f25bcf31ca 100644 --- a/package/MDAnalysis/lib/_distopia.py +++ b/package/MDAnalysis/lib/_distopia.py @@ -36,9 +36,7 @@ else: HAS_DISTOPIA = True -from .c_distances import ( - calc_bond_distance_triclinic as _calc_bond_distance_triclinic_serial, -) + import warnings import numpy as np @@ -46,7 +44,7 @@ def calc_bond_distance_ortho( coords1, coords2: np.ndarray, box: np.ndarray, results: np.ndarray ) -> None: - distopia.calc_bonds_ortho_float( + distopia.calc_bonds_ortho( coords1, coords2, box[:3], results=results ) # upcast is currently required, change for 3.0, see #3927 @@ -55,7 +53,7 @@ def calc_bond_distance_ortho( def calc_bond_distance( coords1: np.ndarray, coords2: np.ndarray, results: np.ndarray ) -> None: - distopia.calc_bonds_no_box_float( + distopia.calc_bonds_no_box( coords1, coords2, results=results ) # upcast is currently required, change for 3.0, see #3927 @@ -64,8 +62,69 @@ def calc_bond_distance( def calc_bond_distance_triclinic( coords1: np.ndarray, coords2: np.ndarray, box: np.ndarray, results: np.ndarray ) -> None: - # redirect to serial backend - warnings.warn( - "distopia does not support triclinic boxes, using serial backend instead." - ) - _calc_bond_distance_triclinic_serial(coords1, coords2, box, results) + distopia.calc_bonds_triclinic(coords1, coords2, box, results=results) + + +def calc_angle( + coords1: np.ndarray, coords2: np.ndarray, coords3: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_angles_no_box(coords1, coords2, coords3, results=results) + +def calc_angle_ortho( + coords1: np.ndarray, coords2: np.ndarray, coords3: np.ndarray, box: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_angles_ortho(coords1, coords2, coords3, box[:3], results=results) + + +def calc_angle_triclinic( + coords1: np.ndarray, coords2: np.ndarray, coords3: np.ndarray, box: np.ndarray, results: np.ndarray +) -> None: + + distopia.calc_angles_triclinic(coords1, coords2, coords3, box, results=results) + + +def calc_dihedral( + coords1: np.ndarray, coords2: np.ndarray, coords3: np.ndarray, coords4: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_dihedrals_no_box(coords1, coords2, coords3, coords4, results=results) + +def calc_dihedral_ortho( + coords1: np.ndarray, coords2: np.ndarray, coords3: np.ndarray, coords4: np.ndarray, box: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_dihedrals_ortho(coords1, coords2, coords3, coords4, box[:3], results=results) + +def calc_dihedral_triclinic( + coords1: np.ndarray, coords2: np.ndarray, coords3: np.ndarray, coords4: np.ndarray, box: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_dihedrals_triclinic(coords1, coords2, coords3, coords4, box, results=results) + +def calc_calc_distance_array( + coords1: np.ndarray, coords2: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_distance_array_no_box(coords1, coords2, results=results) + +def calc_distance_array_ortho( + coords1: np.ndarray, coords2: np.ndarray, box: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_distance_array_ortho(coords1, coords2, box[:3], results=results) + +def calc_distance_array_triclinic( + coords1: np.ndarray, coords2: np.ndarray, box: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_distance_array_triclinic(coords1, coords2, box, results=results) + +def calc_self_distance_array( + coords: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_self_distance_array_no_box(coords, results=results) + +def calc_self_distance_array_ortho( + coords: np.ndarray, box: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_self_distance_array_ortho(coords, box[:3], results=results) + +def calc_self_distance_array_triclinic( + coords: np.ndarray, box: np.ndarray, results: np.ndarray +) -> None: + distopia.calc_self_distance_array_triclinic(coords, box, results=results) + From f8d1295cde9d04d91bf59f6dda5c87f9d609bf89 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Tue, 15 Oct 2024 16:41:55 +1100 Subject: [PATCH 03/30] add backend conditionals --- package/MDAnalysis/lib/_distopia.py | 2 +- .../MDAnalysisTests/lib/test_distances.py | 36 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/package/MDAnalysis/lib/_distopia.py b/package/MDAnalysis/lib/_distopia.py index 4f25bcf31ca..5e05f100dda 100644 --- a/package/MDAnalysis/lib/_distopia.py +++ b/package/MDAnalysis/lib/_distopia.py @@ -98,7 +98,7 @@ def calc_dihedral_triclinic( ) -> None: distopia.calc_dihedrals_triclinic(coords1, coords2, coords3, coords4, box, results=results) -def calc_calc_distance_array( +def calc_distance_array( coords1: np.ndarray, coords2: np.ndarray, results: np.ndarray ) -> None: distopia.calc_distance_array_no_box(coords1, coords2, results=results) diff --git a/testsuite/MDAnalysisTests/lib/test_distances.py b/testsuite/MDAnalysisTests/lib/test_distances.py index 2ef53bab427..29211525427 100644 --- a/testsuite/MDAnalysisTests/lib/test_distances.py +++ b/testsuite/MDAnalysisTests/lib/test_distances.py @@ -963,7 +963,7 @@ def test_bonds_single_coords(self, shift, periodic, backend): @pytest.mark.parametrize("dtype", (np.float32, np.float64)) @pytest.mark.parametrize("pos", ["positions", "positions_atomgroups"]) - @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_angles(self, backend, dtype, pos, request): a, b, c, d = request.getfixturevalue(pos) a, b, c, d = convert_position_dtype_if_ndarray(a, b, c, d, dtype) @@ -979,7 +979,7 @@ def test_angles(self, backend, dtype, pos, request): assert_almost_equal(angles[3], 0.098174833, self.prec, err_msg="Small angle failed in calc_angles") - @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_angles_bad_result(self, positions, backend): a, b, c, d = positions badresult = np.zeros(len(a) - 1) # Bad result array @@ -1005,7 +1005,7 @@ def test_angles_bad_result(self, positions, backend): ) @pytest.mark.parametrize("shift", shifts) @pytest.mark.parametrize("periodic", [True, False]) - @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_angles_single_coords(self, case, shift, periodic, backend): def manual_angle(x, y, z): return mdamath.angle(y - x, y - z) @@ -1026,7 +1026,7 @@ def manual_angle(x, y, z): @pytest.mark.parametrize("dtype", (np.float32, np.float64)) @pytest.mark.parametrize("pos", ["positions", "positions_atomgroups"]) - @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_dihedrals(self, backend, dtype, pos, request): a, b, c, d = request.getfixturevalue(pos) a, b, c, d = convert_position_dtype_if_ndarray(a, b, c, d, dtype) @@ -1039,7 +1039,7 @@ def test_dihedrals(self, backend, dtype, pos, request): assert_almost_equal(dihedrals[3], -0.50714064, self.prec, err_msg="arbitrary dihedral angle failed") - @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_dihedrals_wronglength(self, positions, wronglength, backend): a, b, c, d = positions with pytest.raises(ValueError): @@ -1054,7 +1054,7 @@ def test_dihedrals_wronglength(self, positions, wronglength, backend): with pytest.raises(ValueError): distances.calc_dihedrals(a, b, c, wronglength, backend=backend) - @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_dihedrals_bad_result(self, positions, backend): a, b, c, d = positions badresult = np.zeros(len(a) - 1) # Bad result array @@ -1105,7 +1105,7 @@ def test_dihedrals_bad_result(self, positions, backend): ) @pytest.mark.parametrize("shift", shifts) @pytest.mark.parametrize("periodic", [True, False]) - @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_dihedrals_single_coords(self, case, shift, periodic, backend): def manual_dihedral(a, b, c, d): return mdamath.dihedral(b - a, c - b, d - c) @@ -1472,7 +1472,7 @@ def test_input_unchanged_calc_bonds_atomgroup( assert_equal([crd.positions for crd in crds], refs) @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_input_unchanged_calc_angles(self, coords, box, backend): crds = coords[:3] refs = [crd.copy() for crd in crds] @@ -1481,7 +1481,7 @@ def test_input_unchanged_calc_angles(self, coords, box, backend): assert_equal(crds, refs) @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_input_unchanged_calc_angles_atomgroup(self, coords_atomgroups, box, backend): crds = coords_atomgroups[:3] @@ -1491,7 +1491,7 @@ def test_input_unchanged_calc_angles_atomgroup(self, coords_atomgroups, assert_equal([crd.positions for crd in crds], refs) @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_input_unchanged_calc_dihedrals(self, coords, box, backend): crds = coords refs = [crd.copy() for crd in crds] @@ -1500,7 +1500,7 @@ def test_input_unchanged_calc_dihedrals(self, coords, box, backend): assert_equal(crds, refs) @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_input_unchanged_calc_dihedrals_atomgroup(self, coords_atomgroups, box, backend): crds = coords_atomgroups @@ -1555,14 +1555,14 @@ def empty_coord(): return np.empty((0, 3), dtype=np.float32) @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_empty_input_distance_array(self, empty_coord, box, backend): res = distances.distance_array(empty_coord, empty_coord, box=box, backend=backend) assert_equal(res, np.empty((0, 0), dtype=np.float64)) @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_empty_input_self_distance_array(self, empty_coord, box, backend): res = distances.self_distance_array(empty_coord, box=box, backend=backend) @@ -1620,14 +1620,14 @@ def test_empty_input_calc_bonds(self, empty_coord, box, backend): assert_equal(res, np.empty((0,), dtype=np.float64)) @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_empty_input_calc_angles(self, empty_coord, box, backend): res = distances.calc_angles(empty_coord, empty_coord, empty_coord, box=box, backend=backend) assert_equal(res, np.empty((0,), dtype=np.float64)) @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_empty_input_calc_dihedrals(self, empty_coord, box, backend): res = distances.calc_dihedrals(empty_coord, empty_coord, empty_coord, empty_coord, box=box, backend=backend) @@ -1783,7 +1783,7 @@ def test_output_type_calc_bonds(self, incoords, box, backend): @pytest.mark.parametrize('box', boxes) @pytest.mark.parametrize('incoords', [3 * [coords[0]]] + list(comb(coords[1:], 3))) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_output_type_calc_angles(self, incoords, box, backend): res = distances.calc_angles(*incoords, box=box, backend=backend) maxdim = max([crd.ndim for crd in incoords]) @@ -1798,7 +1798,7 @@ def test_output_type_calc_angles(self, incoords, box, backend): @pytest.mark.parametrize('box', boxes) @pytest.mark.parametrize('incoords', [4 * [coords[0]]] + list(comb(coords[1:], 4))) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_output_type_calc_dihedrals(self, incoords, box, backend): res = distances.calc_dihedrals(*incoords, box=box, backend=backend) maxdim = max([crd.ndim for crd in incoords]) @@ -1812,7 +1812,7 @@ def test_output_type_calc_dihedrals(self, incoords, box, backend): @pytest.mark.parametrize('box', boxes[:2]) @pytest.mark.parametrize('incoords', coords) - @pytest.mark.parametrize('backend', ['serial', 'openmp']) + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_output_type_apply_PBC(self, incoords, box, backend): res = distances.apply_PBC(incoords, box, backend=backend) assert type(res) == np.ndarray From badc778b4ed497179ae0f94a79ffc5e3af20c8ca Mon Sep 17 00:00:00 2001 From: hmacdope Date: Tue, 15 Oct 2024 17:02:52 +1100 Subject: [PATCH 04/30] fix wrong backend --- testsuite/MDAnalysisTests/lib/test_distances.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/MDAnalysisTests/lib/test_distances.py b/testsuite/MDAnalysisTests/lib/test_distances.py index 29211525427..a96888d6199 100644 --- a/testsuite/MDAnalysisTests/lib/test_distances.py +++ b/testsuite/MDAnalysisTests/lib/test_distances.py @@ -1812,7 +1812,7 @@ def test_output_type_calc_dihedrals(self, incoords, box, backend): @pytest.mark.parametrize('box', boxes[:2]) @pytest.mark.parametrize('incoords', coords) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize('backend', ['serial', 'openmp']) def test_output_type_apply_PBC(self, incoords, box, backend): res = distances.apply_PBC(incoords, box, backend=backend) assert type(res) == np.ndarray From e97317b69a947200dc7f59043e65ac3eb758fbc9 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Tue, 15 Oct 2024 17:18:24 +1100 Subject: [PATCH 05/30] shims --- package/MDAnalysis/lib/distances.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index 600bc6cfc46..e79c44c7cdc 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -1494,13 +1494,14 @@ def calc_bonds(coords1: Union[npt.NDArray, 'AtomGroup'], """ numatom = coords1.shape[0] bondlengths = _check_result_array(result, (numatom,)) + if backend == 'distopia': + bondlengths = bondlengths.astype(np.float32) + box = np.asarray(box).astype(np.float32) if box is not None else None if numatom > 0: if box is not None: boxtype, box = check_box(box) if boxtype == "ortho": - if backend == 'distopia': - bondlengths = bondlengths.astype(np.float32) _run( "calc_bond_distance_ortho", args=(coords1, coords2, box, bondlengths), @@ -1608,6 +1609,10 @@ def calc_angles(coords1: Union[npt.NDArray, 'AtomGroup'], numatom = coords1.shape[0] angles = _check_result_array(result, (numatom,)) + if backend == 'distopia': + angles = angles.astype(np.float32) + box = np.asarray(box).astype(np.float32) if box is not None else None + if numatom > 0: if box is not None: boxtype, box = check_box(box) @@ -1624,6 +1629,8 @@ def calc_angles(coords1: Union[npt.NDArray, 'AtomGroup'], args=(coords1, coords2, coords3, angles), backend=backend) + if backend == 'distopia': + angles = angles.astype(np.float64) return angles @@ -1726,6 +1733,10 @@ def calc_dihedrals(coords1: Union[npt.NDArray, 'AtomGroup'], numatom = coords1.shape[0] dihedrals = _check_result_array(result, (numatom,)) + if backend == 'distopia': + dihedrals = dihedrals.astype(np.float32) + box = np.asarray(box).astype(np.float32) if box is not None else None + if numatom > 0: if box is not None: boxtype, box = check_box(box) @@ -1741,7 +1752,8 @@ def calc_dihedrals(coords1: Union[npt.NDArray, 'AtomGroup'], _run("calc_dihedral", args=(coords1, coords2, coords3, coords4, dihedrals), backend=backend) - + if backend == 'distopia': + dihedrals = dihedrals.astype(np.float64) return dihedrals From e0ce653edffce6fe646a30927d4f5d0986d5302f Mon Sep 17 00:00:00 2001 From: hmacdope Date: Tue, 15 Oct 2024 20:42:15 +1100 Subject: [PATCH 06/30] working except for dihedrals --- package/MDAnalysis/lib/distances.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index e79c44c7cdc..847c371a60a 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -313,6 +313,11 @@ def distance_array(reference: Union[npt.NDArray, 'AtomGroup'], distances = _check_result_array(result, (refnum, confnum)) if len(distances) == 0: return distances + + if backend == 'distopia': + distances = distances.astype(np.float32) + box = np.asarray(box).astype(np.float32) if box is not None else None + if box is not None: boxtype, box = check_box(box) if boxtype == 'ortho': @@ -328,6 +333,9 @@ def distance_array(reference: Union[npt.NDArray, 'AtomGroup'], args=(reference, configuration, distances), backend=backend) + if backend == 'distopia': + distances = distances.astype(np.float64) + return distances From 63162dfbf036c46517a3e7ddf8893a0b1ea30909 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Wed, 16 Oct 2024 21:07:32 +1100 Subject: [PATCH 07/30] move to 0.3.0 --- .github/actions/setup-deps/action.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/setup-deps/action.yaml b/.github/actions/setup-deps/action.yaml index 97112b09159..4241d44ecbb 100644 --- a/.github/actions/setup-deps/action.yaml +++ b/.github/actions/setup-deps/action.yaml @@ -59,7 +59,7 @@ inputs: dask: default: 'dask' distopia: - default: 'distopia>=0.2.0' + default: 'distopia>=0.3.0' h5py: default: 'h5py>=2.10' hole2: From 0ec20571b99bee98e67f1912397adf230f2a87c1 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Wed, 16 Oct 2024 21:38:29 +1100 Subject: [PATCH 08/30] bump ci From 2392101a00f17d09592fd05cf0e11441e2205d21 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Thu, 17 Oct 2024 12:27:14 +1100 Subject: [PATCH 09/30] bump ci From 2cb04d2908d87bffce9b293ac3733f04619cc9f3 Mon Sep 17 00:00:00 2001 From: Hugo MacDermott-Opeskin Date: Mon, 21 Oct 2024 11:11:59 +1100 Subject: [PATCH 10/30] Update package/MDAnalysis/lib/distances.py Co-authored-by: Oliver Beckstein --- package/MDAnalysis/lib/distances.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index 847c371a60a..7f75974a58b 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -110,7 +110,7 @@ .. versionchanged:: 2.5.0 Interface to the `distopia`_ package added. .. versionchanged:: 2.7.0 - Distopia support greatly expanded. + Distopia support greatly expanded (with distopia ≥ 0.3.0). Functions --------- From 517c676eb646da569f636269f79fd8a09dbbe560 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Mon, 21 Oct 2024 15:56:40 +1100 Subject: [PATCH 11/30] fix versioning --- .github/actions/setup-deps/action.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/setup-deps/action.yaml b/.github/actions/setup-deps/action.yaml index cbfd91df7e1..aaa4b13ce1b 100644 --- a/.github/actions/setup-deps/action.yaml +++ b/.github/actions/setup-deps/action.yaml @@ -59,7 +59,7 @@ inputs: dask: default: 'dask' distopia: - default: 'distopia>=0.2.0,<0.3.0' + default: 'distopia>=0.3.1' h5py: default: 'h5py>=2.10' hole2: From 36de301ef1281b8ecaf08a0cbcc428f68e325f8c Mon Sep 17 00:00:00 2001 From: hmacdope Date: Mon, 21 Oct 2024 16:05:47 +1100 Subject: [PATCH 12/30] add version checking --- package/MDAnalysis/lib/_distopia.py | 30 ++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/package/MDAnalysis/lib/_distopia.py b/package/MDAnalysis/lib/_distopia.py index 28002393663..fa594b92a15 100644 --- a/package/MDAnalysis/lib/_distopia.py +++ b/package/MDAnalysis/lib/_distopia.py @@ -28,6 +28,9 @@ as a selectable backend. """ import warnings +from packaging.version import Version + +MIN_DISTOPIA_VERSION = Version("0.3.1") # check for distopia try: @@ -37,17 +40,22 @@ else: HAS_DISTOPIA = True - # check for compatibility: currently needs to be >=0.2.0,<0.3.0 (issue - # #4740) No distopia.__version__ available so we have to do some probing. - needed_funcs = ['calc_bonds_no_box_float', 'calc_bonds_ortho_float'] - has_distopia_020 = all([hasattr(distopia, func) for func in needed_funcs]) - if not has_distopia_020: - warnings.warn("Install 'distopia>=0.2.0,<0.3.0' to be used with this " - "release of MDAnalysis. Your installed version of " - "distopia >=0.3.0 will NOT be used.", - category=RuntimeWarning) - del distopia - HAS_DISTOPIA = False + # check for compatibility: currently needs to be >=0.3.1, + + # some versions of `distopia` don't have a version attribute + try: + distopia_version = Version(distopia.__version__) + except AttributeError: + warnings.warn("distopia version cannot be determined, assuming 0.0.0") + distopia_version = Version("0.0.0") + else: + if distopia_version < MIN_DISTOPIA_VERSION: + warnings.warn( + f"distopia version {distopia_version} is too old; " + f"need at least {MIN_DISTOPIA_VERSION}, Your installed version of " + "distopia will NOT be used." + ) + HAS_DISTOPIA = False from .c_distances import ( From b49b02828d4da0f8a97f11f025ddd6bf5821a211 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Mon, 21 Oct 2024 16:06:00 +1100 Subject: [PATCH 13/30] remove erroneous triclinic note --- package/MDAnalysis/lib/distances.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index 7f75974a58b..3b085a75cd5 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -82,12 +82,8 @@ 'distopia' for the `backend` keyword argument. If the distopia backend is selected the `distopia` library will be used to calculate the distances. Note that for functions listed in this table **distopia is not the default backend -if and must be selected.** +and must be selected.** -.. Note:: - Distopia does not currently support triclinic simulation boxes. If you - specify `distopia` as the backend and your simulation box is triclinic, - the function will fall back to the default `serial` backend. .. Note:: Due to the use of Instruction Set Architecture (`ISA`_) specific SIMD From d895d999220925bf795d7b6ccb2cbc044ccf236e Mon Sep 17 00:00:00 2001 From: hmacdope Date: Mon, 21 Oct 2024 16:06:20 +1100 Subject: [PATCH 14/30] remove unnesecary import --- package/MDAnalysis/lib/_distopia.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/package/MDAnalysis/lib/_distopia.py b/package/MDAnalysis/lib/_distopia.py index fa594b92a15..ada6b9fa49f 100644 --- a/package/MDAnalysis/lib/_distopia.py +++ b/package/MDAnalysis/lib/_distopia.py @@ -58,9 +58,6 @@ HAS_DISTOPIA = False -from .c_distances import ( - calc_bond_distance_triclinic as _calc_bond_distance_triclinic_serial, -) import numpy as np From 5733a74bec223f58f82220387c8bd5f62f7c0bf5 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Mon, 21 Oct 2024 16:11:28 +1100 Subject: [PATCH 15/30] add explanatory notes --- package/MDAnalysis/lib/distances.py | 35 +++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index 3b085a75cd5..ce5e1771599 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -311,6 +311,9 @@ def distance_array(reference: Union[npt.NDArray, 'AtomGroup'], return distances if backend == 'distopia': + # distopia requires that all the input arrays are the same type, + # while MDAnalysis allows for mixed types, this should be changed + # pre 0.3.0 release see issue #3707 distances = distances.astype(np.float32) box = np.asarray(box).astype(np.float32) if box is not None else None @@ -330,6 +333,8 @@ def distance_array(reference: Union[npt.NDArray, 'AtomGroup'], backend=backend) if backend == 'distopia': + # mda expects the result to be in float64, so we need to convert it back + # to float64, change for 3.0, see #3707 distances = distances.astype(np.float64) return distances @@ -400,6 +405,14 @@ def self_distance_array(reference: Union[npt.NDArray, 'AtomGroup'], distances = _check_result_array(result, (distnum,)) if len(distances) == 0: return distances + + if backend == 'distopia': + # distopia requires that all the input arrays are the same type, + # while MDAnalysis allows for mixed types, this should be changed + # pre 0.3.0 release see issue #3707 + distances = distances.astype(np.float32) + box = np.asarray(box).astype(np.float32) if box is not None else None + if box is not None: boxtype, box = check_box(box) if boxtype == 'ortho': @@ -415,6 +428,11 @@ def self_distance_array(reference: Union[npt.NDArray, 'AtomGroup'], args=(reference, distances), backend=backend) + if backend == 'distopia': + # mda expects the result to be in float64, so we need to convert it back + # to float64, change for 3.0, see #3707 + distances = distances.astype(np.float64) + return distances @@ -1499,6 +1517,9 @@ def calc_bonds(coords1: Union[npt.NDArray, 'AtomGroup'], numatom = coords1.shape[0] bondlengths = _check_result_array(result, (numatom,)) if backend == 'distopia': + # distopia requires that all the input arrays are the same type, + # while MDAnalysis allows for mixed types, this should be changed + # pre 0.3.0 release see issue #3707 bondlengths = bondlengths.astype(np.float32) box = np.asarray(box).astype(np.float32) if box is not None else None @@ -1518,14 +1539,14 @@ def calc_bonds(coords1: Union[npt.NDArray, 'AtomGroup'], backend=backend, ) else: - if backend == 'distopia': - bondlengths = bondlengths.astype(np.float32) _run( "calc_bond_distance", args=(coords1, coords2, bondlengths), backend=backend, ) if backend == 'distopia': + # mda expects the result to be in float64, so we need to convert it back + # to float64, change for 3.0, see #3707 bondlengths = bondlengths.astype(np.float64) return bondlengths @@ -1614,6 +1635,9 @@ def calc_angles(coords1: Union[npt.NDArray, 'AtomGroup'], angles = _check_result_array(result, (numatom,)) if backend == 'distopia': + # distopia requires that all the input arrays are the same type, + # while MDAnalysis allows for mixed types, this should be changed + # pre 0.3.0 release see issue #3707 angles = angles.astype(np.float32) box = np.asarray(box).astype(np.float32) if box is not None else None @@ -1634,6 +1658,8 @@ def calc_angles(coords1: Union[npt.NDArray, 'AtomGroup'], backend=backend) if backend == 'distopia': + # mda expects the result to be in float64, so we need to convert it back + # to float64, change for 3.0, see #3707 angles = angles.astype(np.float64) return angles @@ -1738,6 +1764,9 @@ def calc_dihedrals(coords1: Union[npt.NDArray, 'AtomGroup'], dihedrals = _check_result_array(result, (numatom,)) if backend == 'distopia': + # distopia requires that all the input arrays are the same type, + # while MDAnalysis allows for mixed types, this should be changed + # pre 0.3.0 release see issue #3707 dihedrals = dihedrals.astype(np.float32) box = np.asarray(box).astype(np.float32) if box is not None else None @@ -1757,6 +1786,8 @@ def calc_dihedrals(coords1: Union[npt.NDArray, 'AtomGroup'], args=(coords1, coords2, coords3, coords4, dihedrals), backend=backend) if backend == 'distopia': + # mda expects the result to be in float64, so we need to convert it back + # to float64, change for 3.0, see #3707 dihedrals = dihedrals.astype(np.float64) return dihedrals From 36c010a9b0f0b1fa0897377c7dd0de1fec8d19b7 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Mon, 21 Oct 2024 16:50:30 +1100 Subject: [PATCH 16/30] change tests in line with discussion --- package/MDAnalysis/lib/_distopia.py | 17 +++---- .../MDAnalysisTests/lib/test_distances.py | 50 +++++++------------ 2 files changed, 25 insertions(+), 42 deletions(-) diff --git a/package/MDAnalysis/lib/_distopia.py b/package/MDAnalysis/lib/_distopia.py index ada6b9fa49f..a467efee2a8 100644 --- a/package/MDAnalysis/lib/_distopia.py +++ b/package/MDAnalysis/lib/_distopia.py @@ -37,25 +37,24 @@ import distopia except ImportError: HAS_DISTOPIA = False + distopia_version = Version("0.0.0") else: HAS_DISTOPIA = True # check for compatibility: currently needs to be >=0.3.1, - # some versions of `distopia` don't have a version attribute try: distopia_version = Version(distopia.__version__) except AttributeError: warnings.warn("distopia version cannot be determined, assuming 0.0.0") distopia_version = Version("0.0.0") - else: - if distopia_version < MIN_DISTOPIA_VERSION: - warnings.warn( - f"distopia version {distopia_version} is too old; " - f"need at least {MIN_DISTOPIA_VERSION}, Your installed version of " - "distopia will NOT be used." - ) - HAS_DISTOPIA = False + if distopia_version < MIN_DISTOPIA_VERSION: + warnings.warn( + f"distopia version {distopia_version} is too old; " + f"need at least {MIN_DISTOPIA_VERSION}, Your installed version of " + "distopia will NOT be used.", category=RuntimeWarning + ) + HAS_DISTOPIA = False import numpy as np diff --git a/testsuite/MDAnalysisTests/lib/test_distances.py b/testsuite/MDAnalysisTests/lib/test_distances.py index d2f987815ae..a718482106b 100644 --- a/testsuite/MDAnalysisTests/lib/test_distances.py +++ b/testsuite/MDAnalysisTests/lib/test_distances.py @@ -21,7 +21,7 @@ # J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787 # import sys -from unittest.mock import Mock, patch +from unittest.mock import patch import pytest import numpy as np from numpy.testing import assert_equal, assert_almost_equal, assert_allclose @@ -840,38 +840,19 @@ def convert_position_dtype_if_ndarray(a, b, c, d, dtype): -def test_HAS_DISTOPIA_incompatible_distopia(): - # warn if distopia is the wrong version and set HAS_DISTOPIA to False + +def test_HAS_DISTOPIA_distopia_too_old(): + # mock a version of distopia that is too old sys.modules.pop("distopia", None) - sys.modules.pop("MDAnalysis.lib._distopia", None) - - # fail any Attribute access for calc_bonds_ortho_float, - # calc_bonds_no_box_float but pretend to have the distopia - # 0.3.0 functions (from - # https://github.com/MDAnalysis/distopia/blob/main/distopia/__init__.py - # __all__): - mock_distopia_030 = Mock(spec=[ - 'calc_bonds_ortho', - 'calc_bonds_no_box', - 'calc_bonds_triclinic', - 'calc_angles_no_box', - 'calc_angles_ortho', - 'calc_angles_triclinic', - 'calc_dihedrals_no_box', - 'calc_dihedrals_ortho', - 'calc_dihedrals_triclinic', - 'calc_distance_array_no_box', - 'calc_distance_array_ortho', - 'calc_distance_array_triclinic', - 'calc_self_distance_array_no_box', - 'calc_self_distance_array_ortho', - 'calc_self_distance_array_triclinic', - ]) - with patch.dict("sys.modules", {"distopia": mock_distopia_030}): - with pytest.warns(RuntimeWarning, - match="Install 'distopia>=0.2.0,<0.3.0' to"): - import MDAnalysis.lib._distopia - assert not MDAnalysis.lib._distopia.HAS_DISTOPIA + sys.modules.pop("MDAnalysis.lib._distopia", None) + if HAS_DISTOPIA: + with patch('distopia.__version__', '0.1.0'): + with pytest.warns(RuntimeWarning, + match="distopia will NOT be used"): + import MDAnalysis.lib._distopia + assert not MDAnalysis.lib._distopia.HAS_DISTOPIA + + class TestCythonFunctions(object): # Unit tests for calc_bonds calc_angles and calc_dihedrals in lib.distances @@ -1071,7 +1052,8 @@ def test_dihedrals(self, backend, dtype, pos, request): assert_equal(len(dihedrals), 4, err_msg="calc_dihedrals results have wrong length") assert np.isnan(dihedrals[0]), "Zero length dihedral failed" assert np.isnan(dihedrals[1]), "Straight line dihedral failed" - assert_almost_equal(dihedrals[2], np.pi, self.prec, err_msg="180 degree dihedral failed") + # 180 degree dihedral can be either pi or (-pi for distopia) + assert_almost_equal(np.abs(dihedrals[2]), np.pi, self.prec, err_msg="180 degree dihedral failed") assert_almost_equal(dihedrals[3], -0.50714064, self.prec, err_msg="arbitrary dihedral angle failed") @@ -1201,6 +1183,8 @@ def test_numpy_compliance_dihedrals(self, positions, backend): bc = b - c cd = c - d dihedrals_numpy = np.array([mdamath.dihedral(x, y, z) for x, y, z in zip(ab, bc, cd)]) + # 180 (and 360) degree dihedral can be either pi or (-pi for distopia) + dihedrals[2] = np.abs(dihedrals[2]) assert_almost_equal(dihedrals, dihedrals_numpy, self.prec, err_msg="Cython dihedrals didn't match numpy calculations") From 763b49365e67b05b75f4cf15ec4aac04e01cb648 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Mon, 21 Oct 2024 16:53:01 +1100 Subject: [PATCH 17/30] changelog --- package/CHANGELOG | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 85a7208627b..6a78f095dee 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -18,7 +18,7 @@ The rules for this file: ljwoods2, aditya292002, pstaerk, PicoCentauri, BFedder, tyler.je.reddy, SampurnaM, leonwehrhan, kainszs, orionarcher, yuxuanzhuang, PythonFZ, laksh-krishna-sharma, orbeckst, MattTDavies, - talagayev, aya9aladdin + talagayev, aya9aladdin, hmacdope * 2.8.0 @@ -59,6 +59,8 @@ Fixes * Fix groups.py doctests using sphinx directives (Issue #3925, PR #4374) Enhancements + * Improve distopia backend support in line with new functionality available + in `distopia 0.3.0+` * Removed type and mass guessing from all topology parsers (PR #3753) * Added guess_TopologyAttrs() API to the Universe to handle attribute guessing (PR #3753) From 10356f773beeb1757a59de4614e4c985c6bd4136 Mon Sep 17 00:00:00 2001 From: Hugo MacDermott-Opeskin Date: Thu, 24 Oct 2024 09:39:21 +1100 Subject: [PATCH 18/30] Apply suggestions from code review Co-authored-by: Oliver Beckstein --- package/CHANGELOG | 2 +- package/MDAnalysis/lib/distances.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 6a78f095dee..e8337b2f5f4 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -60,7 +60,7 @@ Fixes Enhancements * Improve distopia backend support in line with new functionality available - in `distopia 0.3.0+` + in distopia >= 0.3.1 (PR #4734) * Removed type and mass guessing from all topology parsers (PR #3753) * Added guess_TopologyAttrs() API to the Universe to handle attribute guessing (PR #3753) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index ce5e1771599..6e1656da366 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -105,8 +105,8 @@ :class:`~MDAnalysis.core.groups.AtomGroup` or an :class:`np.ndarray` .. versionchanged:: 2.5.0 Interface to the `distopia`_ package added. -.. versionchanged:: 2.7.0 - Distopia support greatly expanded (with distopia ≥ 0.3.0). +.. versionchanged:: 2.8.0 + Distopia support greatly expanded (with distopia ≥ 0.3.1). Functions --------- From 3b0024ab2e7ff29e588dabd283c951a58df14b7b Mon Sep 17 00:00:00 2001 From: hmacdope Date: Thu, 24 Oct 2024 09:46:34 +1100 Subject: [PATCH 19/30] add more tests --- .../MDAnalysisTests/lib/test_distances.py | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/testsuite/MDAnalysisTests/lib/test_distances.py b/testsuite/MDAnalysisTests/lib/test_distances.py index a718482106b..a2955cbaf40 100644 --- a/testsuite/MDAnalysisTests/lib/test_distances.py +++ b/testsuite/MDAnalysisTests/lib/test_distances.py @@ -438,7 +438,7 @@ def Triclinic_Universe(): universe = MDAnalysis.Universe(TRIC) return universe -@pytest.mark.parametrize('backend', ['serial', 'openmp']) +@pytest.mark.parametrize('backend', distopia_conditional_backend()) class TestDistanceArrayDCD_TRIC(object): # reasonable precision so that tests succeed on 32 and 64 bit machines # (the reference values were obtained on 64 bit) @@ -555,7 +555,7 @@ def test_atomgroup_matches_numpy_tric(self, Triclinic_Universe, backend, err_msg="AtomGroup and NumPy distances do not match") -@pytest.mark.parametrize('backend', ['serial', 'openmp']) +@pytest.mark.parametrize('backend', distopia_conditional_backend()) class TestSelfDistanceArrayDCD_TRIC(object): prec = 5 @@ -652,7 +652,6 @@ def test_atomgroup_matches_numpy_tric(self, Triclinic_Universe, backend, err_msg="AtomGroup and NumPy distances do not match") -@pytest.mark.parametrize('backend', ['serial', 'openmp']) class TestTriclinicDistances(object): """Unit tests for the Triclinic PBC functions. Tests: @@ -696,6 +695,7 @@ def S_mol_single(TRIC): S_mol2 = TRIC.atoms[390].position return S_mol1, S_mol2 + @pytest.mark.parametrize('backend', ['serial', 'openmp']) @pytest.mark.parametrize('S_mol', [S_mol, S_mol_single], indirect=True) def test_transforms(self, S_mol, tri_vec_box, box, backend): # To check the cython coordinate transform, the same operation is done in numpy @@ -717,9 +717,11 @@ def test_transforms(self, S_mol, tri_vec_box, box, backend): assert_almost_equal(S_test1, S_mol1, self.prec, err_msg="Round trip 1 failed in transform") assert_almost_equal(S_test2, S_mol2, self.prec, err_msg="Round trip 2 failed in transform") + + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_selfdist(self, S_mol, box, tri_vec_box, backend): S_mol1, S_mol2 = S_mol - R_coords = distances.transform_StoR(S_mol1, box, backend=backend) + R_coords = distances.transform_StoR(S_mol1, box, backend="serial") # Transform functions are tested elsewhere so taken as working here dists = distances.self_distance_array(R_coords, box=box, backend=backend) # Manually calculate self_distance_array @@ -739,7 +741,7 @@ def test_selfdist(self, S_mol, box, tri_vec_box, backend): err_msg="self_distance_array failed with input 1") # Do it again for input 2 (has wider separation in points) - R_coords = distances.transform_StoR(S_mol2, box, backend=backend) + R_coords = distances.transform_StoR(S_mol2, box, backend="serial") # Transform functions are tested elsewhere so taken as working here dists = distances.self_distance_array(R_coords, box=box, backend=backend) # Manually calculate self_distance_array @@ -758,11 +760,13 @@ def test_selfdist(self, S_mol, box, tri_vec_box, backend): assert_almost_equal(dists, manual, self.prec, err_msg="self_distance_array failed with input 2") + + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_distarray(self, S_mol, tri_vec_box, box, backend): S_mol1, S_mol2 = S_mol - R_mol1 = distances.transform_StoR(S_mol1, box, backend=backend) - R_mol2 = distances.transform_StoR(S_mol2, box, backend=backend) + R_mol1 = distances.transform_StoR(S_mol1, box, backend="serial") + R_mol2 = distances.transform_StoR(S_mol2, box, backend="serial") # Try with box dists = distances.distance_array(R_mol1, R_mol2, box=box, backend=backend) @@ -780,6 +784,8 @@ def test_distarray(self, S_mol, tri_vec_box, box, backend): assert_almost_equal(dists, manual, self.prec, err_msg="distance_array failed with box") + + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_pbc_dist(self, S_mol, box, backend): S_mol1, S_mol2 = S_mol results = np.array([[37.629944]]) @@ -788,6 +794,8 @@ def test_pbc_dist(self, S_mol, box, backend): assert_almost_equal(dists, results, self.prec, err_msg="distance_array failed to retrieve PBC distance") + + @pytest.mark.parametrize('backend', distopia_conditional_backend()) def test_pbc_wrong_wassenaar_distance(self, backend): box = [2, 2, 2, 60, 60, 60] tri_vec_box = mdamath.triclinic_vectors(box) From 9c149ec98980544e1116779ed596e20aa5090700 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Thu, 24 Oct 2024 09:49:57 +1100 Subject: [PATCH 20/30] improve docs --- package/MDAnalysis/lib/distances.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index 6e1656da366..53eef44cc8a 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -48,6 +48,7 @@ "OpenMP" :mod:`c_distances_openmp` parallel implementation in C/Cython with OpenMP + "distopia" `_distopia` SIMD-accelerated implementation ========== ========================= ====================================== Use of the distopia library @@ -279,7 +280,7 @@ def distance_array(reference: Union[npt.NDArray, 'AtomGroup'], ``numpy.float64``. Avoids creating the array which saves time when the function is called repeatedly. - backend : {'serial', 'OpenMP'}, optional + backend : {'serial', 'OpenMP', 'distopia'}, optional Keyword selecting the type of acceleration. Returns @@ -370,7 +371,7 @@ def self_distance_array(reference: Union[npt.NDArray, 'AtomGroup'], Preallocated result array which must have the shape ``(n*(n-1)/2,)`` and dtype ``numpy.float64``. Avoids creating the array which saves time when the function is called repeatedly. - backend : {'serial', 'OpenMP'}, optional + backend : {'serial', 'OpenMP', 'distopia'}, optional Keyword selecting the type of acceleration. Returns @@ -1607,7 +1608,7 @@ def calc_angles(coords1: Union[npt.NDArray, 'AtomGroup'], Preallocated result array of dtype ``numpy.float64`` and shape ``(n,)`` (for ``n`` coordinate triplets). Avoids recreating the array in repeated function calls. - backend : {'serial', 'OpenMP'}, optional + backend : {'serial', 'OpenMP', 'distopia'}, optional Keyword selecting the type of acceleration. Returns @@ -1732,7 +1733,7 @@ def calc_dihedrals(coords1: Union[npt.NDArray, 'AtomGroup'], Preallocated result array of dtype ``numpy.float64`` and shape ``(n,)`` (for ``n`` coordinate quadruplets). Avoids recreating the array in repeated function calls. - backend : {'serial', 'OpenMP'}, optional + backend : {'serial', 'OpenMP', 'distopia'}, optional Keyword selecting the type of acceleration. Returns From 8cae6f8c1b5a51bd8cf8eae8d59372ea3bff1205 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Thu, 24 Oct 2024 09:55:09 +1100 Subject: [PATCH 21/30] improve docs again --- package/MDAnalysis/lib/distances.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index 53eef44cc8a..debb840a71f 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -298,6 +298,8 @@ def distance_array(reference: Union[npt.NDArray, 'AtomGroup'], .. versionchanged:: 2.3.0 Can now accept an :class:`~MDAnalysis.core.groups.AtomGroup` as an argument in any position and checks inputs using type hinting. + .. versionchanged:: 2.8.0 + Added support for the `distopia` backend. """ confnum = configuration.shape[0] refnum = reference.shape[0] @@ -395,6 +397,8 @@ def self_distance_array(reference: Union[npt.NDArray, 'AtomGroup'], .. versionchanged:: 2.3.0 Can now accept an :class:`~MDAnalysis.core.groups.AtomGroup` as an argument in any position and checks inputs using type hinting. + .. versionchanged:: 2.8.0 + Added support for the `distopia` backend. """ refnum = reference.shape[0] distnum = refnum * (refnum - 1) // 2 From 50efeb960e9b133b7a735d834e796fa23472da9a Mon Sep 17 00:00:00 2001 From: hmacdope Date: Thu, 24 Oct 2024 14:42:14 +1100 Subject: [PATCH 22/30] hmmm --- package/MDAnalysis/lib/_distopia.py | 5 +++-- testsuite/MDAnalysisTests/lib/test_distances.py | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/package/MDAnalysis/lib/_distopia.py b/package/MDAnalysis/lib/_distopia.py index a467efee2a8..f442ed9d19b 100644 --- a/package/MDAnalysis/lib/_distopia.py +++ b/package/MDAnalysis/lib/_distopia.py @@ -66,7 +66,7 @@ def calc_bond_distance_ortho( distopia.calc_bonds_ortho( coords1, coords2, box[:3], results=results ) - # upcast is currently required, change for 3.0, see #3927 + def calc_bond_distance( @@ -75,7 +75,6 @@ def calc_bond_distance( distopia.calc_bonds_no_box( coords1, coords2, results=results ) - # upcast is currently required, change for 3.0, see #3927 def calc_bond_distance_triclinic( @@ -89,6 +88,7 @@ def calc_angle( ) -> None: distopia.calc_angles_no_box(coords1, coords2, coords3, results=results) + def calc_angle_ortho( coords1: np.ndarray, coords2: np.ndarray, coords3: np.ndarray, box: np.ndarray, results: np.ndarray ) -> None: @@ -122,6 +122,7 @@ def calc_distance_array( ) -> None: distopia.calc_distance_array_no_box(coords1, coords2, results=results) + def calc_distance_array_ortho( coords1: np.ndarray, coords2: np.ndarray, box: np.ndarray, results: np.ndarray ) -> None: diff --git a/testsuite/MDAnalysisTests/lib/test_distances.py b/testsuite/MDAnalysisTests/lib/test_distances.py index a2955cbaf40..8c35e1c7252 100644 --- a/testsuite/MDAnalysisTests/lib/test_distances.py +++ b/testsuite/MDAnalysisTests/lib/test_distances.py @@ -473,6 +473,8 @@ def test_outarray(self, DCD_Universe, backend): natoms = len(U.atoms) d = np.zeros((natoms, natoms), np.float64) distances.distance_array(x0, x1, result=d, backend=backend) + print("AFTER") + print(d) assert_equal(d.shape, (natoms, natoms), "wrong shape, should be" " (Natoms,Natoms) entries") assert_almost_equal(d.min(), 0.11981228170520701, self.prec, @@ -937,6 +939,19 @@ def test_bonds(self, box, backend, dtype, pos, request): assert_almost_equal(dists_pbc[3], 3.46410072, self.prec, err_msg="PBC check #w with box") + @pytest.mark.parametrize("dtype", (np.float32, np.float64)) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) + def test_results_inplace_all_backends(self, backend, dtype,): + N = 10 + c0 = np.ones(3 * N, dtype=dtype).reshape(N, 3) * 2 + c1 = np.ones(3 * N, dtype=dtype).reshape(N, 3) * 3 + + result = np.zeros(N, dtype=np.float64) + distances.calc_bonds(c0, c1, result=result, backend=backend) + expected = np.ones(N, dtype=dtype) * 3**(1/2) + # test the result array is updated in place + assert_almost_equal(result, expected, self.prec, err_msg="calc_bonds inplace failed") + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_bonds_badbox(self, positions, backend): a, b, c, d = positions From d4b9e9d652fd66e47021d00df7e7c97c63808173 Mon Sep 17 00:00:00 2001 From: Oliver Beckstein Date: Tue, 10 Dec 2024 15:07:31 -0700 Subject: [PATCH 23/30] add type for calc_bond_distance_ortho() --- package/MDAnalysis/lib/_distopia.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/MDAnalysis/lib/_distopia.py b/package/MDAnalysis/lib/_distopia.py index 1335e95b14d..60bbf603ac0 100644 --- a/package/MDAnalysis/lib/_distopia.py +++ b/package/MDAnalysis/lib/_distopia.py @@ -61,7 +61,7 @@ def calc_bond_distance_ortho( - coords1, coords2: np.ndarray, box: np.ndarray, results: np.ndarray + coords1: np.ndarray, coords2: np.ndarray, box: np.ndarray, results: np.ndarray ) -> None: distopia.calc_bonds_ortho( coords1, coords2, box[:3], results=results From 2621552515ccb2d5747d9ddc7a01cd159bb5220c Mon Sep 17 00:00:00 2001 From: hmacdope Date: Tue, 31 Dec 2024 15:29:17 +1100 Subject: [PATCH 24/30] just stuff answers back in if we used a buffer --- package/MDAnalysis/lib/distances.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index debb840a71f..af63cdd31f9 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -1553,6 +1553,9 @@ def calc_bonds(coords1: Union[npt.NDArray, 'AtomGroup'], # mda expects the result to be in float64, so we need to convert it back # to float64, change for 3.0, see #3707 bondlengths = bondlengths.astype(np.float64) + if result is not None: + result[:] = bondlengths + return bondlengths @@ -1666,6 +1669,8 @@ def calc_angles(coords1: Union[npt.NDArray, 'AtomGroup'], # mda expects the result to be in float64, so we need to convert it back # to float64, change for 3.0, see #3707 angles = angles.astype(np.float64) + if result is not None: + result[:] = angles return angles @@ -1794,6 +1799,8 @@ def calc_dihedrals(coords1: Union[npt.NDArray, 'AtomGroup'], # mda expects the result to be in float64, so we need to convert it back # to float64, change for 3.0, see #3707 dihedrals = dihedrals.astype(np.float64) + if result is not None: + result[:] = dihedrals return dihedrals From c75dce2a14a85bce0aa9ac948351035f5ccf2e24 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Tue, 31 Dec 2024 15:45:19 +1100 Subject: [PATCH 25/30] add cram for distance array also --- package/MDAnalysis/lib/distances.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index c26aff50d7a..bdb7f777279 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -374,6 +374,8 @@ def distance_array( # mda expects the result to be in float64, so we need to convert it back # to float64, change for 3.0, see #3707 distances = distances.astype(np.float64) + if result is not None: + result[:] = distances return distances @@ -482,6 +484,8 @@ def self_distance_array( # mda expects the result to be in float64, so we need to convert it back # to float64, change for 3.0, see #3707 distances = distances.astype(np.float64) + if result is not None: + result[:] = distances return distances From 02a3d93426720dedaa68329e9f8bea024b59285e Mon Sep 17 00:00:00 2001 From: hmacdope Date: Tue, 31 Dec 2024 15:49:09 +1100 Subject: [PATCH 26/30] fix changelogs --- package/CHANGELOG | 10 +++++----- package/MDAnalysis/lib/distances.py | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 37d9fcae2a6..710f1992a7f 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -14,15 +14,17 @@ The rules for this file: ------------------------------------------------------------------------------- -??/??/?? IAlibay, ChiahsinChu, RMeli +??/??/?? IAlibay, ChiahsinChu, RMeli, hmacdope * 2.9.0 Fixes Enhancements + * Improve distopia backend support in line with new functionality available + in distopia >= 0.3.1 (PR #4734) * Add check and warning for empty (all zero) coordinates in RDKit converter (PR #4824) - * Added `precision` for XYZWriter (Issue #4775, PR #4771) + * Added `precision` for XYZWriter (Issue #4775, PR #4771) Changes @@ -33,7 +35,7 @@ Deprecations ljwoods2, aditya292002, pstaerk, PicoCentauri, BFedder, tyler.je.reddy, SampurnaM, leonwehrhan, kainszs, orionarcher, yuxuanzhuang, PythonFZ, laksh-krishna-sharma, orbeckst, MattTDavies, - talagayev, aya9aladdin, hmacdope + talagayev, aya9aladdin * 2.8.0 @@ -81,8 +83,6 @@ Fixes * Fix groups.py doctests using sphinx directives (Issue #3925, PR #4374) Enhancements - * Improve distopia backend support in line with new functionality available - in distopia >= 0.3.1 (PR #4734) * Removed type and mass guessing from all topology parsers (PR #3753) * Added guess_TopologyAttrs() API to the Universe to handle attribute guessing (PR #3753) diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py index bdb7f777279..816b5ad9d92 100644 --- a/package/MDAnalysis/lib/distances.py +++ b/package/MDAnalysis/lib/distances.py @@ -106,7 +106,7 @@ :class:`~MDAnalysis.core.groups.AtomGroup` or an :class:`np.ndarray` .. versionchanged:: 2.5.0 Interface to the `distopia`_ package added. -.. versionchanged:: 2.8.0 +.. versionchanged:: 2.9.0 Distopia support greatly expanded (with distopia ≥ 0.3.1). Functions @@ -325,7 +325,7 @@ def distance_array( .. versionchanged:: 2.3.0 Can now accept an :class:`~MDAnalysis.core.groups.AtomGroup` as an argument in any position and checks inputs using type hinting. - .. versionchanged:: 2.8.0 + .. versionchanged:: 2.9.0 Added support for the `distopia` backend. """ confnum = configuration.shape[0] @@ -436,7 +436,7 @@ def self_distance_array( .. versionchanged:: 2.3.0 Can now accept an :class:`~MDAnalysis.core.groups.AtomGroup` as an argument in any position and checks inputs using type hinting. - .. versionchanged:: 2.8.0 + .. versionchanged:: 2.9.0 Added support for the `distopia` backend. """ refnum = reference.shape[0] From 9101d81858f71f913ce08597125a064b4c636d49 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Thu, 2 Jan 2025 18:27:14 +1100 Subject: [PATCH 27/30] black --- .../MDAnalysisTests/lib/test_distances.py | 159 +++++++++++------- 1 file changed, 96 insertions(+), 63 deletions(-) diff --git a/testsuite/MDAnalysisTests/lib/test_distances.py b/testsuite/MDAnalysisTests/lib/test_distances.py index 87c50b71be2..7efd085568c 100644 --- a/testsuite/MDAnalysisTests/lib/test_distances.py +++ b/testsuite/MDAnalysisTests/lib/test_distances.py @@ -46,7 +46,6 @@ def distopia_conditional_backend(): return ["serial", "openmp"] - class TestCheckResultArray(object): ref = np.zeros(1, dtype=np.float64) @@ -527,7 +526,8 @@ def Triclinic_Universe(): universe = MDAnalysis.Universe(TRIC) return universe -@pytest.mark.parametrize('backend', distopia_conditional_backend()) + +@pytest.mark.parametrize("backend", distopia_conditional_backend()) class TestDistanceArrayDCD_TRIC(object): # reasonable precision so that tests succeed on 32 and 64 bit machines # (the reference values were obtained on 64 bit) @@ -571,16 +571,27 @@ def test_outarray(self, DCD_Universe, backend): natoms = len(U.atoms) d = np.zeros((natoms, natoms), np.float64) distances.distance_array(x0, x1, result=d, backend=backend) - # START: DEBUG printing --- remove + # START: DEBUG printing --- remove print("AFTER") print(d) # END: DEBUG printing --- remove - assert_equal(d.shape, (natoms, natoms), "wrong shape, should be" - " (Natoms,Natoms) entries") - assert_almost_equal(d.min(), 0.11981228170520701, self.prec, - err_msg="wrong minimum distance value") - assert_almost_equal(d.max(), 53.572192429459619, self.prec, - err_msg="wrong maximum distance value") + assert_equal( + d.shape, + (natoms, natoms), + "wrong shape, should be" " (Natoms,Natoms) entries", + ) + assert_almost_equal( + d.min(), + 0.11981228170520701, + self.prec, + err_msg="wrong minimum distance value", + ) + assert_almost_equal( + d.max(), + 53.572192429459619, + self.prec, + err_msg="wrong maximum distance value", + ) def test_periodic(self, DCD_Universe, backend): # boring with the current dcd as that has no PBC @@ -693,7 +704,7 @@ def test_atomgroup_matches_numpy_tric( ) -@pytest.mark.parametrize('backend', distopia_conditional_backend()) +@pytest.mark.parametrize("backend", distopia_conditional_backend()) class TestSelfDistanceArrayDCD_TRIC(object): prec = 5 @@ -883,8 +894,8 @@ def S_mol_single(TRIC): S_mol2 = TRIC.atoms[390].position return S_mol1, S_mol2 - @pytest.mark.parametrize('backend', ['serial', 'openmp']) - @pytest.mark.parametrize('S_mol', [S_mol, S_mol_single], indirect=True) + @pytest.mark.parametrize("backend", ["serial", "openmp"]) + @pytest.mark.parametrize("S_mol", [S_mol, S_mol_single], indirect=True) def test_transforms(self, S_mol, tri_vec_box, box, backend): # To check the cython coordinate transform, the same operation is done in numpy # Is a matrix multiplication of Coords x tri_vec_box = NewCoords, so can use np.dot @@ -925,8 +936,7 @@ def test_transforms(self, S_mol, tri_vec_box, box, backend): err_msg="Round trip 2 failed in transform", ) - - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_selfdist(self, S_mol, box, tri_vec_box, backend): S_mol1, S_mol2 = S_mol R_coords = distances.transform_StoR(S_mol1, box, backend="serial") @@ -980,8 +990,7 @@ def test_selfdist(self, S_mol, box, tri_vec_box, backend): err_msg="self_distance_array failed with input 2", ) - - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_distarray(self, S_mol, tri_vec_box, box, backend): S_mol1, S_mol2 = S_mol @@ -1007,8 +1016,7 @@ def test_distarray(self, S_mol, tri_vec_box, box, backend): dists, manual, self.prec, err_msg="distance_array failed with box" ) - - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_pbc_dist(self, S_mol, box, backend): S_mol1, S_mol2 = S_mol results = np.array([[37.629944]]) @@ -1023,8 +1031,7 @@ def test_pbc_dist(self, S_mol, box, backend): err_msg="distance_array failed to retrieve PBC distance", ) - - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_pbc_wrong_wassenaar_distance(self, backend): box = [2, 2, 2, 60, 60, 60] tri_vec_box = mdamath.triclinic_vectors(box) @@ -1084,14 +1091,15 @@ def convert_position_dtype_if_ndarray(a, b, c, d, dtype): def test_HAS_DISTOPIA_distopia_too_old(): # mock a version of distopia that is too old sys.modules.pop("distopia", None) - sys.modules.pop("MDAnalysis.lib._distopia", None) + sys.modules.pop("MDAnalysis.lib._distopia", None) if HAS_DISTOPIA: - with patch('distopia.__version__', '0.1.0'): - with pytest.warns(RuntimeWarning, - match="distopia will NOT be used"): + with patch("distopia.__version__", "0.1.0"): + with pytest.warns( + RuntimeWarning, match="distopia will NOT be used" + ): import MDAnalysis.lib._distopia - assert not MDAnalysis.lib._distopia.HAS_DISTOPIA + assert not MDAnalysis.lib._distopia.HAS_DISTOPIA class TestCythonFunctions(object): @@ -1230,16 +1238,22 @@ def test_bonds(self, box, backend, dtype, pos, request): @pytest.mark.parametrize("dtype", (np.float32, np.float64)) @pytest.mark.parametrize("backend", distopia_conditional_backend()) - def test_results_inplace_all_backends(self, backend, dtype,): + def test_results_inplace_all_backends( + self, + backend, + dtype, + ): N = 10 c0 = np.ones(3 * N, dtype=dtype).reshape(N, 3) * 2 c1 = np.ones(3 * N, dtype=dtype).reshape(N, 3) * 3 result = np.zeros(N, dtype=np.float64) distances.calc_bonds(c0, c1, result=result, backend=backend) - expected = np.ones(N, dtype=dtype) * 3**(1/2) + expected = np.ones(N, dtype=dtype) * 3 ** (1 / 2) # test the result array is updated in place - assert_almost_equal(result, expected, self.prec, err_msg="calc_bonds inplace failed") + assert_almost_equal( + result, expected, self.prec, err_msg="calc_bonds inplace failed" + ) @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_bonds_badbox(self, positions, backend): @@ -1398,9 +1412,18 @@ def test_dihedrals(self, backend, dtype, pos, request): assert np.isnan(dihedrals[0]), "Zero length dihedral failed" assert np.isnan(dihedrals[1]), "Straight line dihedral failed" # 180 degree dihedral can be either pi or (-pi for distopia) - assert_almost_equal(np.abs(dihedrals[2]), np.pi, self.prec, err_msg="180 degree dihedral failed") - assert_almost_equal(dihedrals[3], -0.50714064, self.prec, - err_msg="arbitrary dihedral angle failed") + assert_almost_equal( + np.abs(dihedrals[2]), + np.pi, + self.prec, + err_msg="180 degree dihedral failed", + ) + assert_almost_equal( + dihedrals[3], + -0.50714064, + self.prec, + err_msg="arbitrary dihedral angle failed", + ) @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_dihedrals_wronglength(self, positions, wronglength, backend): @@ -1537,11 +1560,17 @@ def test_numpy_compliance_dihedrals(self, positions, backend): ab = a - b bc = b - c cd = c - d - dihedrals_numpy = np.array([mdamath.dihedral(x, y, z) for x, y, z in zip(ab, bc, cd)]) + dihedrals_numpy = np.array( + [mdamath.dihedral(x, y, z) for x, y, z in zip(ab, bc, cd)] + ) # 180 (and 360) degree dihedral can be either pi or (-pi for distopia) - dihedrals[2] = np.abs(dihedrals[2]) - assert_almost_equal(dihedrals, dihedrals_numpy, self.prec, - err_msg="Cython dihedrals didn't match numpy calculations") + dihedrals[2] = np.abs(dihedrals[2]) + assert_almost_equal( + dihedrals, + dihedrals_numpy, + self.prec, + err_msg="Cython dihedrals didn't match numpy calculations", + ) @pytest.mark.parametrize("backend", ["serial", "openmp"]) @@ -1900,8 +1929,8 @@ def test_input_unchanged_calc_bonds_atomgroup( res = distances.calc_bonds(crds[0], crds[1], box=box, backend=backend) assert_equal([crd.positions for crd in crds], refs) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_input_unchanged_calc_angles(self, coords, box, backend): crds = coords[:3] refs = [crd.copy() for crd in crds] @@ -1910,10 +1939,11 @@ def test_input_unchanged_calc_angles(self, coords, box, backend): ) assert_equal(crds, refs) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) - def test_input_unchanged_calc_angles_atomgroup(self, coords_atomgroups, - box, backend): + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) + def test_input_unchanged_calc_angles_atomgroup( + self, coords_atomgroups, box, backend + ): crds = coords_atomgroups[:3] refs = [crd.positions.copy() for crd in crds] res = distances.calc_angles( @@ -1921,8 +1951,8 @@ def test_input_unchanged_calc_angles_atomgroup(self, coords_atomgroups, ) assert_equal([crd.positions for crd in crds], refs) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_input_unchanged_calc_dihedrals(self, coords, box, backend): crds = coords refs = [crd.copy() for crd in crds] @@ -1931,10 +1961,11 @@ def test_input_unchanged_calc_dihedrals(self, coords, box, backend): ) assert_equal(crds, refs) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) - def test_input_unchanged_calc_dihedrals_atomgroup(self, coords_atomgroups, - box, backend): + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) + def test_input_unchanged_calc_dihedrals_atomgroup( + self, coords_atomgroups, box, backend + ): crds = coords_atomgroups refs = [crd.positions.copy() for crd in crds] res = distances.calc_dihedrals( @@ -1991,16 +2022,16 @@ def empty_coord(): # empty coordinate array: return np.empty((0, 3), dtype=np.float32) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_empty_input_distance_array(self, empty_coord, box, backend): res = distances.distance_array( empty_coord, empty_coord, box=box, backend=backend ) assert_equal(res, np.empty((0, 0), dtype=np.float64)) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_empty_input_self_distance_array(self, empty_coord, box, backend): res = distances.self_distance_array( empty_coord, box=box, backend=backend @@ -2070,16 +2101,16 @@ def test_empty_input_calc_bonds(self, empty_coord, box, backend): ) assert_equal(res, np.empty((0,), dtype=np.float64)) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_empty_input_calc_angles(self, empty_coord, box, backend): res = distances.calc_angles( empty_coord, empty_coord, empty_coord, box=box, backend=backend ) assert_equal(res, np.empty((0,), dtype=np.float64)) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_empty_input_calc_dihedrals(self, empty_coord, box, backend): res = distances.calc_dihedrals( empty_coord, @@ -2258,10 +2289,11 @@ def test_output_type_calc_bonds(self, incoords, box, backend): coord = [crd for crd in incoords if crd.ndim == maxdim][0] assert res.shape == (coord.shape[0],) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('incoords', - [3 * [coords[0]]] + list(comb(coords[1:], 3))) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize( + "incoords", [3 * [coords[0]]] + list(comb(coords[1:], 3)) + ) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_output_type_calc_angles(self, incoords, box, backend): res = distances.calc_angles(*incoords, box=box, backend=backend) maxdim = max([crd.ndim for crd in incoords]) @@ -2273,10 +2305,11 @@ def test_output_type_calc_angles(self, incoords, box, backend): coord = [crd for crd in incoords if crd.ndim == maxdim][0] assert res.shape == (coord.shape[0],) - @pytest.mark.parametrize('box', boxes) - @pytest.mark.parametrize('incoords', - [4 * [coords[0]]] + list(comb(coords[1:], 4))) - @pytest.mark.parametrize('backend', distopia_conditional_backend()) + @pytest.mark.parametrize("box", boxes) + @pytest.mark.parametrize( + "incoords", [4 * [coords[0]]] + list(comb(coords[1:], 4)) + ) + @pytest.mark.parametrize("backend", distopia_conditional_backend()) def test_output_type_calc_dihedrals(self, incoords, box, backend): res = distances.calc_dihedrals(*incoords, box=box, backend=backend) maxdim = max([crd.ndim for crd in incoords]) From 447beac75adc6b50bfefb95ab05eecca36f822d8 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Thu, 2 Jan 2025 18:40:14 +1100 Subject: [PATCH 28/30] imporve mock test --- .../MDAnalysisTests/lib/test_distances.py | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/testsuite/MDAnalysisTests/lib/test_distances.py b/testsuite/MDAnalysisTests/lib/test_distances.py index 7efd085568c..50455e9411b 100644 --- a/testsuite/MDAnalysisTests/lib/test_distances.py +++ b/testsuite/MDAnalysisTests/lib/test_distances.py @@ -27,7 +27,7 @@ from numpy.testing import assert_equal, assert_almost_equal, assert_allclose import itertools from itertools import combinations_with_replacement as comb - +from types import ModuleType import MDAnalysis import numpy as np import pytest @@ -1092,14 +1092,27 @@ def test_HAS_DISTOPIA_distopia_too_old(): # mock a version of distopia that is too old sys.modules.pop("distopia", None) sys.modules.pop("MDAnalysis.lib._distopia", None) - if HAS_DISTOPIA: - with patch("distopia.__version__", "0.1.0"): - with pytest.warns( - RuntimeWarning, match="distopia will NOT be used" - ): - import MDAnalysis.lib._distopia - assert not MDAnalysis.lib._distopia.HAS_DISTOPIA + module_name = "distopia" + mocked_module = ModuleType(module_name) + # too old version + mocked_module.__version__ = "0.1.0" + sys.modules[module_name] = mocked_module + + + import MDAnalysis.lib._distopia + assert not MDAnalysis.lib._distopia.HAS_DISTOPIA + + sys.modules.pop("distopia", None) + sys.modules.pop("MDAnalysis.lib._distopia", None) + + # new enough version + mocked_module.__version__ = "0.4.0" + sys.modules[module_name] = mocked_module + + import MDAnalysis.lib._distopia + assert MDAnalysis.lib._distopia.HAS_DISTOPIA + class TestCythonFunctions(object): From 0a60016053af05c354b1b00e36c1f51807a95cde Mon Sep 17 00:00:00 2001 From: hmacdope Date: Thu, 2 Jan 2025 18:40:59 +1100 Subject: [PATCH 29/30] fix typo --- package/CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index fe05459c379..23b4dbf7ddc 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -15,7 +15,7 @@ The rules for this file: ------------------------------------------------------------------------------- ??/??/?? IAlibay, ChiahsinChu, RMeli, tanishy7777, talagayev, - tylerjereddy. hmacdope + tylerjereddy, hmacdope * 2.9.0 From 730cee9420ccf993476130804030cba3f6f8e707 Mon Sep 17 00:00:00 2001 From: hmacdope Date: Thu, 2 Jan 2025 18:42:40 +1100 Subject: [PATCH 30/30] add debug printing --- testsuite/MDAnalysisTests/lib/test_distances.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/testsuite/MDAnalysisTests/lib/test_distances.py b/testsuite/MDAnalysisTests/lib/test_distances.py index 50455e9411b..e990cf176de 100644 --- a/testsuite/MDAnalysisTests/lib/test_distances.py +++ b/testsuite/MDAnalysisTests/lib/test_distances.py @@ -571,10 +571,6 @@ def test_outarray(self, DCD_Universe, backend): natoms = len(U.atoms) d = np.zeros((natoms, natoms), np.float64) distances.distance_array(x0, x1, result=d, backend=backend) - # START: DEBUG printing --- remove - print("AFTER") - print(d) - # END: DEBUG printing --- remove assert_equal( d.shape, (natoms, natoms),