Skip to content
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
32 changes: 32 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,38 @@ against, and semantic-release manages only the **patch** component.
- `test_a_copy_does_not_pin_its_source` — asserts the aliasing/copying split
directly, via refcount: a view increfs its parent, a copy does not.

### Fixed

- **`vector()` and `matrix()` no longer silently convert their input.** All
eight factory overloads (native and complex, view and copy) take
`.noconvert()`, so the dtype must match a registered one exactly and the
array must be C-contiguous.

The reported symptom was unregistered dtypes — `f16`, `uint16`, `uint32`,
`uint64` — coming back as `DenseVector_f32`. Fixing it turned up something
worse, because nanobind's converting pass repacks **layout and dtype
together** and takes the first overload that converts, which is `float32`:

```python
a = np.array([0.1, 1.0, 0.2, 2.0, 0.3, 3.0]) # float64
v = mtl5.vector(a[::2])
# v is a DenseVector_f32, v.is_view is True, and v[0] == 0.10000000149011612
```

An ordinary float64 slice was **silently downcast to float32** while
reporting `is_view=True` and aliasing nothing. The old test covering this
used values (1.0, 3.0, 5.0) that are exact in float32, which is why it read
as benign.

Both now raise `TypeError`, listing every accepted dtype. `np.ascontiguousarray(a)`
is the deliberate way to ask for the copy, and it keeps float64. This matches
what `mtl5.array.asarray` has always done — its `.noconvert()` carries the
same reasoning, and the factories simply never got it.

**This is a behaviour change.** Code passing a non-contiguous array, or a
dtype outside {f32, f64, i8, i16, i32, i64, u8, c64, c128}, now raises where
it used to return a quietly converted result.

### Changed

- **The build toolchain is pinned**: `cibuildwheel==4.2.0` and `build==1.6.0` in
Expand Down
40 changes: 36 additions & 4 deletions python/src/mtl5_complex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,15 @@ template <typename T>
void register_complex_factories(nb::module_& m) {
m.def("vector", [](nb::ndarray<T, nb::ndim<1>, nb::c_contig, nb::device::cpu> a) {
return VectorView<T>(a.shape(0), a.data(), nb::cast(a));
}, "a"_a, "Create a zero-copy MTL5 vector view of a 1-D complex NumPy array");
}, nb::arg("a").noconvert(),
"Create a zero-copy MTL5 vector view of a 1-D complex NumPy array.\n\n"
"The view aliases the NumPy buffer; writes through either are visible in both.\n"
"The dtype must be complex64 or complex128 exactly -- whichever this\n"
"overload is for -- and the array must be C-contiguous. Anything else\n"
"raises TypeError rather than converting: a converted array is a view\n"
"of a temporary, so it is neither zero-copy nor the dtype you asked\n"
"for, and a real array would silently acquire an imaginary part. Pass\n"
"np.ascontiguousarray(a), or a.astype(...), to convert deliberately.");

m.def("vector_copy", [](nb::ndarray<T, nb::ndim<1>, nb::c_contig, nb::device::cpu> a) {
const std::size_t n = a.shape(0);
Expand All @@ -327,11 +335,27 @@ void register_complex_factories(nb::module_& m) {
for (std::size_t i = 0; i < n; ++i) v[i] = src[i];
}
return VectorView<T>(std::move(v));
}, "a"_a, "Create an owning MTL5 vector (copies from a complex NumPy array)");
}, nb::arg("a").noconvert(),
"Create an owning MTL5 vector (copies from a complex NumPy array).\n\n"
"The dtype must be complex64 or complex128 exactly -- whichever this\n"
"overload is for -- and the array must be C-contiguous. Anything else\n"
"raises TypeError rather than converting. This copies the DATA but not\n"
"the TYPE, so a silent conversion would still hand back a container of\n"
"the wrong precision, and a real array would quietly acquire an\n"
"imaginary part. Pass np.ascontiguousarray(a), or a.astype(...), to\n"
"convert deliberately.");

m.def("matrix", [](nb::ndarray<T, nb::ndim<2>, nb::c_contig, nb::device::cpu> a) {
return MatrixView<T>(a.shape(0), a.shape(1), a.data(), nb::cast(a));
}, "a"_a, "Create a zero-copy MTL5 matrix view of a 2-D complex NumPy array");
}, nb::arg("a").noconvert(),
"Create a zero-copy MTL5 matrix view of a 2-D complex NumPy array.\n\n"
"The view aliases the NumPy buffer; writes through either are visible in both.\n"
"The dtype must be complex64 or complex128 exactly -- whichever this\n"
"overload is for -- and the array must be C-contiguous. Anything else\n"
"raises TypeError rather than converting: a converted array is a view\n"
"of a temporary, so it is neither zero-copy nor the dtype you asked\n"
"for, and a real array would silently acquire an imaginary part. Pass\n"
"np.ascontiguousarray(a), or a.astype(...), to convert deliberately.");

m.def("matrix_copy", [](nb::ndarray<T, nb::ndim<2>, nb::c_contig, nb::device::cpu> a) {
const std::size_t r = a.shape(0), c = a.shape(1);
Expand All @@ -344,7 +368,15 @@ void register_complex_factories(nb::module_& m) {
M(i, j) = src[i * c + j];
}
return MatrixView<T>(std::move(M));
}, "a"_a, "Create an owning MTL5 matrix (copies from a complex NumPy array)");
}, nb::arg("a").noconvert(),
"Create an owning MTL5 matrix (copies from a complex NumPy array).\n\n"
"The dtype must be complex64 or complex128 exactly -- whichever this\n"
"overload is for -- and the array must be C-contiguous. Anything else\n"
"raises TypeError rather than converting. This copies the DATA but not\n"
"the TYPE, so a silent conversion would still hand back a container of\n"
"the wrong precision, and a real array would quietly acquire an\n"
"imaginary part. Pass np.ascontiguousarray(a), or a.astype(...), to\n"
"convert deliberately.");
}

// ---------------------------------------------------------------------------
Expand Down
30 changes: 26 additions & 4 deletions python/src/mtl5_module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,14 @@ void register_native_vector_factory(nb::module_& m) {
// Zero-copy: borrow memory from the NumPy array
m.def("vector", [](nb::ndarray<T, nb::ndim<1>, nb::c_contig, nb::device::cpu> a) {
return VectorView<T>(a.shape(0), a.data(), nb::cast(a));
}, "a"_a, "Create a zero-copy MTL5 vector view of a 1-D NumPy array");
}, nb::arg("a").noconvert(),
"Create a zero-copy MTL5 vector view of a 1-D NumPy array.\n\n"
"The dtype must match a registered one exactly and the array must be\n"
"C-contiguous. Anything else raises TypeError rather than converting,\n"
"because a converted array is a view of a temporary: neither zero-copy\n"
"nor the dtype you asked for. A float64 slice such as a[::2] used to\n"
"come back as a float32 vector reporting is_view=True. Pass\n"
"np.ascontiguousarray(a), or a.astype(...), to say so deliberately.");

// Explicit copy variant
m.def("vector_copy", [](nb::ndarray<T, nb::ndim<1>, nb::c_contig, nb::device::cpu> a) {
Expand All @@ -238,7 +245,11 @@ void register_native_vector_factory(nb::module_& m) {
v[i] = src[i];
}
return VectorView<T>(std::move(v));
}, "a"_a, "Create an owning MTL5 vector (copies data from NumPy array)");
}, nb::arg("a").noconvert(),
"Create an owning MTL5 vector (copies data from a NumPy array).\n\n"
"Exact dtype and C-contiguity are required here too: this copies the\n"
"data but not the TYPE, so a silent conversion would still hand back a\n"
"vector of the wrong precision.");
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -296,7 +307,14 @@ void register_native_matrix_factory(nb::module_& m) {
// Zero-copy: borrow memory from the NumPy array
m.def("matrix", [](nb::ndarray<T, nb::ndim<2>, nb::c_contig, nb::device::cpu> a) {
return MatrixView<T>(a.shape(0), a.shape(1), a.data(), nb::cast(a));
}, "a"_a, "Create a zero-copy MTL5 matrix view of a 2-D NumPy array");
}, nb::arg("a").noconvert(),
"Create a zero-copy MTL5 matrix view of a 2-D NumPy array.\n\n"
"The dtype must match a registered one exactly and the array must be\n"
"C-contiguous. Anything else raises TypeError rather than converting,\n"
"because a converted array is a view of a temporary: neither zero-copy\n"
"nor the dtype you asked for. A float64 slice such as a[::2] used to\n"
"come back as a float32 matrix reporting is_view=True. Pass\n"
"np.ascontiguousarray(a), or a.astype(...), to say so deliberately.");

// Explicit copy variant
m.def("matrix_copy", [](nb::ndarray<T, nb::ndim<2>, nb::c_contig, nb::device::cpu> a) {
Expand All @@ -310,7 +328,11 @@ void register_native_matrix_factory(nb::module_& m) {
M(r, c) = src[r * cols + c];
}
return MatrixView<T>(std::move(M));
}, "a"_a, "Create an owning MTL5 matrix (copies data from NumPy array)");
}, nb::arg("a").noconvert(),
"Create an owning MTL5 matrix (copies data from a NumPy array).\n\n"
"Exact dtype and C-contiguity are required here too: this copies the\n"
"data but not the TYPE, so a silent conversion would still hand back a\n"
"matrix of the wrong precision.");
}

// ---------------------------------------------------------------------------
Expand Down
78 changes: 71 additions & 7 deletions tests/test_vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,19 +204,83 @@ def test_existing_types_keep_their_matmul(self):


class TestNonContiguous:
def test_non_contiguous_implicitly_copied(self):
"""nanobind implicitly copies non-contiguous arrays to make them contiguous.
The resulting view is NOT shared with the original (safe behavior)."""
"""A non-contiguous array is rejected rather than silently repacked.

This used to be accepted, and what it did was worse than the old docstring
said. nanobind's converting pass repacks layout AND dtype together, and it
takes the first overload that converts — which is float32. So a float64
slice came back as a *float32* vector, losing precision, while reporting
`is_view=True` and not aliasing anything:

a = np.array([0.1, 1.0, 0.2, 2.0, 0.3, 3.0])
mtl5.vector(a[::2])[0] -> 0.10000000149011612 (exact f64: 0.1)

The values in the old test (1.0, 3.0, 5.0) are exact in float32, which is
why it never noticed. `.noconvert()` on the factories makes this a
TypeError; `np.ascontiguousarray` is the deliberate way to ask for the copy.
"""

def test_non_contiguous_is_rejected(self):
a = np.array([1.0, 2.0, 3.0, 4.0, 5.0, 6.0])
with pytest.raises(TypeError):
mtl5.vector(a[::2])

def test_ascontiguousarray_is_the_way_through(self):
a = np.array([1.0, 2.0, 3.0, 4.0, 5.0, 6.0])
sliced = a[::2] # non-contiguous: [1, 3, 5]
v = mtl5.vector(sliced)
v = mtl5.vector(np.ascontiguousarray(a[::2]))
assert isinstance(v, mtl5.DenseVector_f64), "and it keeps float64"
assert len(v) == 3
assert v[0] == pytest.approx(1.0)
assert v[1] == pytest.approx(3.0)
# Mutation does NOT propagate back to original (implicit copy)
# An explicit copy, so mutation does not reach the source -- same
# end behaviour as before, but now the caller asked for it.
v[0] = 99.0
assert a[0] == 1.0

def test_a_float64_slice_no_longer_becomes_float32(self):
"""The regression this closes: silent precision loss on ordinary
slicing. 0.1 is not representable in float32."""
a = np.array([0.1, 1.0, 0.2, 2.0, 0.3, 3.0], dtype=np.float64)
with pytest.raises(TypeError):
mtl5.vector(a[::2])
v = mtl5.vector(np.ascontiguousarray(a[::2]))
assert v[0] == 0.1, "float64 must survive the round trip exactly"


class TestFactoryRejectsUnregisteredDtypes:
"""`.noconvert()` also stops unregistered dtypes being silently converted.

Before, `mtl5.vector(np.arange(4, dtype=np.float16))` returned a
DenseVector_f32 reporting is_view=True — a view of the converted temporary,
so writes through the NumPy array were invisible.
"""

@pytest.mark.parametrize(
"dt",
[np.float16, np.uint16, np.uint32, np.uint64],
ids=["f16", "u16", "u32", "u64"],
)
def test_unregistered_dtype_raises(self, dt):
with pytest.raises(TypeError):
mtl5.vector(np.arange(4, dtype=dt))

@pytest.mark.parametrize(
"dt",
[
np.float32,
np.float64,
np.int8,
np.int16,
np.int32,
np.int64,
np.uint8,
np.complex64,
np.complex128,
],
)
def test_registered_dtypes_still_dispatch_exactly(self, dt):
v = mtl5.vector(np.arange(4, dtype=dt))
assert v.to_numpy().dtype == np.dtype(dt)


class TestNorm:
def test_l2_norm_f64(self):
Expand Down