Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAINT: shims for array copy semantics #4482

Merged
merged 3 commits into from
Apr 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package/MDAnalysis/analysis/helix_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def vector_of_best_fit(coordinates):
"""
centered = coordinates - coordinates.mean(axis=0)
Mt_M = np.matmul(centered.T, centered)
u, s, vh = np.linalg.linalg.svd(Mt_M)
u, s, vh = np.linalg.svd(Mt_M)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking - how backwards compatible is this? Do we need to try except an import or will this work with older versions of numpy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure we're safe since I tested the runtime downgrade to our oldest supported version (1.23.2) without issue.

If you don't believe me, you could use the version switcher in the NumPy docs and it is there in version 1.13: https://numpy.org/doc/1.13/reference/generated/numpy.linalg.svd.html

That's pretty ancient now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So sorry @tylerjereddy - I completely missed this from your previous comment message (it's the second time in a row over two PRs really sorry about this).

vector = vh[0]

# does vector face first local helix origin?
Expand Down
6 changes: 4 additions & 2 deletions package/MDAnalysis/lib/formats/cython_util.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ cimport numpy as cnp
from libc.stdlib cimport free
from cpython cimport PyObject, Py_INCREF

from MDAnalysis.lib.util import no_copy_shim

cnp.import_array()


Expand Down Expand Up @@ -71,7 +73,7 @@ cdef class ArrayWrapper:
self.data_type = data_type
self.ndim = ndim

def __array__(self):
def __array__(self, dtype=None, copy=None):
""" Here we use the __array__ method, that is called when numpy
tries to get an array from the object."""
ndarray = cnp.PyArray_SimpleNewFromData(self.ndim,
Expand Down Expand Up @@ -110,7 +112,7 @@ cdef cnp.ndarray ptr_to_ndarray(void* data_ptr, cnp.int64_t[:] dim, int data_typ
array_wrapper = ArrayWrapper()
array_wrapper.set_data(<void*> data_ptr, <int*> &dim[0], dim.size, data_type)

cdef cnp.ndarray ndarray = np.array(array_wrapper, copy=False)
cdef cnp.ndarray ndarray = np.array(array_wrapper, copy=no_copy_shim)
# Assign our object to the 'base' of the ndarray object
ndarray[:] = array_wrapper.__array__()
# Increment the reference count, as the above assignement was done in
Expand Down
13 changes: 7 additions & 6 deletions package/MDAnalysis/lib/transformations.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@
from numpy.linalg import norm

from .mdamath import angle as vecangle
from MDAnalysis.lib.util import no_copy_shim

def identity_matrix():
"""Return 4x4 identity/unit matrix.
Expand Down Expand Up @@ -326,7 +327,7 @@ def rotation_matrix(angle, direction, point=None):
M[:3, :3] = R
if point is not None:
# rotation not around origin
point = np.array(point[:3], dtype=np.float64, copy=False)
point = np.array(point[:3], dtype=np.float64, copy=no_copy_shim)
M[:3, 3] = point - np.dot(R, point)
return M

Expand Down Expand Up @@ -497,7 +498,7 @@ def projection_matrix(point, normal, direction=None,

"""
M = np.identity(4)
point = np.array(point[:3], dtype=np.float64, copy=False)
point = np.array(point[:3], dtype=np.float64, copy=no_copy_shim)
normal = unit_vector(normal[:3])
if perspective is not None:
# perspective projection
Expand All @@ -515,7 +516,7 @@ def projection_matrix(point, normal, direction=None,
M[3, 3] = np.dot(perspective, normal)
elif direction is not None:
# parallel projection
direction = np.array(direction[:3], dtype=np.float64, copy=False)
direction = np.array(direction[:3], dtype=np.float64, copy=no_copy_shim)
scale = np.dot(direction, normal)
M[:3, :3] -= np.outer(direction, normal) / scale
M[:3, 3] = direction * (np.dot(point, normal) / scale)
Expand Down Expand Up @@ -970,8 +971,8 @@ def superimposition_matrix(v0, v1, scaling=False, usesvd=True):
True

"""
v0 = np.array(v0, dtype=np.float64, copy=False)[:3]
v1 = np.array(v1, dtype=np.float64, copy=False)[:3]
v0 = np.array(v0, dtype=np.float64, copy=no_copy_shim)[:3]
v1 = np.array(v1, dtype=np.float64, copy=no_copy_shim)[:3]

if v0.shape != v1.shape or v0.shape[1] < 3:
raise ValueError("vector sets are of wrong shape or type")
Expand Down Expand Up @@ -1314,7 +1315,7 @@ def quaternion_from_matrix(matrix, isprecise=False):
True

"""
M = np.array(matrix, dtype=np.float64, copy=False)[:4, :4]
M = np.array(matrix, dtype=np.float64, copy=no_copy_shim)[:4, :4]
if isprecise:
q = np.empty((4, ), dtype=np.float64)
t = np.trace(M)
Expand Down
8 changes: 8 additions & 0 deletions package/MDAnalysis/lib/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2552,3 +2552,11 @@
self._kwargs[key] = arg
return func(self, *args, **kwargs)
return wrapper


def no_copy_shim():
if np.lib.NumpyVersion >= "2.0.0rc1":
copy = None

Check warning on line 2559 in package/MDAnalysis/lib/util.py

View check run for this annotation

Codecov / codecov/patch

package/MDAnalysis/lib/util.py#L2559

Added line #L2559 was not covered by tests
else:
copy = False
return copy

Check warning on line 2562 in package/MDAnalysis/lib/util.py

View check run for this annotation

Codecov / codecov/patch

package/MDAnalysis/lib/util.py#L2561-L2562

Added lines #L2561 - L2562 were not covered by tests
Loading