Skip to content

Commit 5e969c9

Browse files
mhorsdalCopilotrobertodrPanadestein
authored
fix: šŸ› validate chi_out, cutoff, and boundary rank in apply()/compress() (#38)
* fix: šŸ› validate chi_out, cutoff, and boundary rank in apply()/compress() Both functions previously let malformed arguments reach deep into the sweep before failing, if they failed at all: - chi_out=0: no error, returns a network with every bond dimension 0. - chi_out=-3: raised, but a generic NumPy "negative dimensions are not allowed" ValueError that never names chi_out. A non-integer chi_out (e.g. 2.5) similarly raised an unnamed, opaque TypeError. - cutoff=2.0: no error, every bond silently collapses to rank 1. - cutoff=-1.0: no error, silently treated as "no truncation". - infer_kind only inspects the first site tensor, so a train with inconsistent boundary ranks (which is valid-looking at the first site but not at the last) or a periodic-boundary train whose rank happens to match a valid one (e.g. a periodic MPS, indistinguishable from an open MPO by first-site rank alone) was accepted and only failed later, deep in the sweep. Add _validation.py with validate_chi_out, validate_cutoff, and validate_boundary_rank, and call them at the top of apply() and compress(), before any array allocation or device transfer, so all three now raise a targeted ValueError immediately. Update tests/test_package.py: - test_compress_unsupported_type now uses a rank-1 array instead of rank-4, since rank-4 input is now caught by validate_boundary_rank with a more specific error rather than the generic TypeError this test checks for. - Add coverage for chi_out, cutoff, and boundary-rank validation. Known limitation: a periodic-boundary train of exactly two sites is not caught. Its boundary tensors have no interior tensor to compare against, and their rank alone (e.g. 3 for a periodic MPS) can be identical to a valid two-site MPO's, making it a genuine ambiguity rather than a missed check. Closes #15 * fix: improve type checking in _validation.py Apply batched suggestions from code review: - validate_chi_out now rejects `bool` - validate_cutoff now ensures cutoff is real Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Mats Horsdal <38429082+mhorsdal@users.noreply.github.com> * fix: šŸ› raise TypeError for a non-integer chi_out Assisted-by: pi:claude-sonnet-5.5 --------- Signed-off-by: Mats Horsdal <38429082+mhorsdal@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Roberto Di Remigio EikĆ„s <robertodr@users.noreply.github.com> Co-authored-by: Panadestein <rpana92@gmail.com>
1 parent 19dc88c commit 5e969c9

6 files changed

Lines changed: 221 additions & 17 deletions

File tree

ā€Žsrc/src_method/_tensor_train.pyā€Ž

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import numpy as np
2222
from opt_einsum import contract
2323

24+
from ._validation import validate_open_boundary
2425
from .utils import to_numpy
2526

2627
if TYPE_CHECKING:
@@ -210,8 +211,9 @@ def normalize_stack(
210211
the contracted train.
211212
212213
Raises:
213-
ValueError: If the stack is empty, if the trains differ in length, or if
214-
adjacent trains have mismatched physical dimensions.
214+
ValueError: If the stack is empty, if a train is not open-boundary, if the
215+
trains differ in length, or if adjacent trains have mismatched physical
216+
dimensions.
215217
TypeError: If a train has an unrecognised layout or an MPS sits anywhere
216218
other than at one end of the stack.
217219
"""
@@ -220,6 +222,10 @@ def normalize_stack(
220222
raise ValueError(msg)
221223
kinds: list[TrainKind | None] = [infer_kind(train) for train in trains]
222224
_check_roles(kinds)
225+
for i, (train, kind) in enumerate(zip(trains, kinds)):
226+
validate_open_boundary(
227+
train, _MPS_BOUNDARY_NDIM if kind == "mps" else _MPO_BOUNDARY_NDIM, i
228+
)
223229
sizes = [len(train) for train in trains]
224230
if len(set(sizes)) > 1:
225231
msg = f"All tensor trains must have the same number of sites, got {sizes}."
@@ -237,7 +243,8 @@ def _check_roles(kinds: Sequence[TrainKind | None]) -> None:
237243
if None in kinds:
238244
msg = (
239245
f"Unsupported tensor network layout for train {kinds.index(None)}: "
240-
"expected an MPS or MPO given as a list of per-site arrays."
246+
"expected an MPS or MPO given as a list of per-site arrays "
247+
"(periodic boundary conditions are not supported)."
241248
)
242249
raise TypeError(msg)
243250
if len(kinds) > 1 and ("mps" in kinds[1:-1] or kinds[0] == kinds[-1] == "mps"):
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""Argument and layout checks shared by the public entry points.
2+
3+
A malformed call should fail at once with a message naming the culprit, rather than
4+
as an opaque shape error deep inside the sweep.
5+
"""
6+
7+
from __future__ import annotations
8+
9+
import numbers
10+
from typing import TYPE_CHECKING
11+
12+
import numpy as np
13+
14+
if TYPE_CHECKING:
15+
from collections.abc import Sequence
16+
17+
from numpy.typing import NDArray
18+
19+
__all__ = ["validate_chi_out", "validate_cutoff", "validate_open_boundary"]
20+
21+
22+
def validate_chi_out(chi_out: int) -> None:
23+
"""Reject a non-integer or non-positive output bond dimension.
24+
25+
Any `numbers.Integral` is accepted, so NumPy integers pass; `bool` does not.
26+
27+
Args:
28+
chi_out: The requested output bond dimension.
29+
30+
Raises:
31+
TypeError: If `chi_out` is not an integer.
32+
ValueError: If `chi_out` is not positive.
33+
"""
34+
if isinstance(chi_out, bool) or not isinstance(chi_out, numbers.Integral):
35+
msg = f"chi_out must be an integer, got {chi_out!r}."
36+
raise TypeError(msg)
37+
if chi_out < 1:
38+
msg = f"chi_out must be positive, got {chi_out!r}."
39+
raise ValueError(msg)
40+
41+
42+
def validate_cutoff(cutoff: float) -> None:
43+
"""Reject a cutoff outside ``[0.0, 1.0)``.
44+
45+
A cutoff of 1 or more would discard every singular value, and a negative one
46+
would silently mean "no truncation".
47+
48+
Args:
49+
cutoff: The relative singular-value cutoff.
50+
51+
Raises:
52+
ValueError: If `cutoff` is not a real number in ``[0.0, 1.0)``.
53+
"""
54+
if not isinstance(cutoff, numbers.Real) or not 0.0 <= cutoff < 1.0:
55+
msg = f"cutoff must be in [0.0, 1.0), got {cutoff!r}."
56+
raise ValueError(msg)
57+
58+
59+
def validate_open_boundary(
60+
train: Sequence[NDArray], boundary_ndim: int, index: int
61+
) -> None:
62+
"""Check that a train has the layout of an open-boundary MPS or MPO.
63+
64+
An open-boundary train has rank ``boundary_ndim`` at its two ends and one more
65+
(the second bond) at every interior site. A periodic train has the interior rank
66+
everywhere; read from its first site it is misclassified as a train of the
67+
next-lower rank, which this check unmasks.
68+
69+
A two-site train has no interior site, so a periodic one cannot be told apart
70+
from an open one of the same rank and is not caught.
71+
72+
Args:
73+
train: The site arrays of the train.
74+
boundary_ndim: The rank of the boundary sites, as inferred from the first
75+
site.
76+
index: The position of the train in the stack, for the error message.
77+
78+
Raises:
79+
ValueError: If the ranks do not follow the open-boundary pattern.
80+
"""
81+
expected = [boundary_ndim, *[boundary_ndim + 1] * (len(train) - 2), boundary_ndim]
82+
ranks = [np.ndim(site) for site in train]
83+
if len(train) > 1 and ranks != expected:
84+
msg = (
85+
f"Train {index} has site ranks {ranks}, expected {expected}: only "
86+
"open boundary conditions are supported."
87+
)
88+
raise ValueError(msg)

ā€Žsrc/src_method/apply.pyā€Ž

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,13 @@ def apply(
5959
The site arrays of the compressed tensor network (MPS or MPO).
6060
6161
Raises:
62-
TypeError: If the combination of input tensor types is unsupported.
63-
ValueError: If the two trains differ in length or in the physical
64-
dimensions they join, if a sub-three-site train is not exactly two
65-
sites, or if ``device`` is not recognised.
62+
TypeError: If ``chi_out`` is not an integer or the combination of input
63+
tensor types is unsupported.
64+
ValueError: If ``chi_out`` is not positive, if ``cutoff`` is not
65+
in ``[0.0, 1.0)``, if a train is not open-boundary, if the two trains
66+
differ in length or in the physical dimensions they join, if a
67+
sub-three-site train is not exactly two sites, or if ``device`` is not
68+
recognised.
6669
ImportError: If ``device="gpu"`` but cupy is not installed.
6770
"""
6871
left_kind = infer_kind(left_tensor)

ā€Žsrc/src_method/compress.pyā€Ž

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,12 @@ def compress(
5555
The site arrays of the compressed tensor network (MPS or MPO).
5656
5757
Raises:
58-
TypeError: If the input tensor type is unsupported.
59-
ValueError: If a sub-three-site train is not exactly two sites, or if
60-
``device`` is not recognised.
58+
TypeError: If ``chi_out`` is not an integer or the input tensor type is
59+
unsupported.
60+
ValueError: If ``chi_out`` is not positive, if ``cutoff`` is not
61+
in ``[0.0, 1.0)``, if the train is not open-boundary, if a
62+
sub-three-site train is not exactly two sites, or if ``device`` is not
63+
recognised.
6164
ImportError: If ``device="gpu"`` but cupy is not installed.
6265
"""
6366
return src(

ā€Žsrc/src_method/stack.pyā€Ž

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
exact_stack,
1919
normalize_stack,
2020
)
21+
from ._validation import validate_chi_out, validate_cutoff
2122
from .utils import default_rng, get_xp, setup_logging, sketch_dtype
2223

2324
if TYPE_CHECKING:
@@ -87,13 +88,17 @@ def src(
8788
form, as numpy arrays (host-side, whatever the ``device``).
8889
8990
Raises:
90-
TypeError: If a train has an unrecognised layout or an MPS sits anywhere
91-
other than at one end of the stack.
92-
ValueError: If the stack is empty, if the trains differ in length or in
93-
the physical dimensions they join, if a sub-three-site stack is not
94-
exactly two sites, or if ``device`` is not recognised.
91+
TypeError: If ``chi_out`` is not an integer, if a train has an unrecognised
92+
layout or an MPS sits anywhere other than at one end of the stack.
93+
ValueError: If ``chi_out`` is not positive, if ``cutoff`` is not in
94+
``[0.0, 1.0)``, if a train is not open-boundary, if the stack is empty,
95+
if the trains differ in length or in the physical dimensions they join,
96+
if a sub-three-site stack is not exactly two sites, or if ``device`` is
97+
not recognised.
9598
ImportError: If ``device="gpu"`` but cupy is not installed.
9699
"""
100+
validate_chi_out(chi_out)
101+
validate_cutoff(cutoff)
97102
xp = get_xp(device)
98103
prng = default_rng(seed)
99104
layers, kind = normalize_stack(trains)

ā€Žtests/test_package.pyā€Ž

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,13 +573,111 @@ def test_apply_unsupported_types(n_sites, phys_dim, chi_out, array_type):
573573
def test_compress_unsupported_type(n_sites, chi_out, array_type):
574574
"""Tests that compress raises TypeError for unsupported tensor layouts."""
575575

576-
# Rank-4 boundary tensors are neither an MPS nor an MPO (e.g. a PEPS row)
576+
# Rank-4 boundary tensors are neither an MPS nor an MPO (e.g. a PEPS row, or a
577+
# periodic MPO); the message points at the periodic case.
577578
peps_like = [np.zeros((2, 2, 2, 2), dtype=array_type)] * n_sites
578579

579-
with pytest.raises(TypeError):
580+
with pytest.raises(TypeError, match="periodic"):
580581
compress(peps_like, chi_out=chi_out, dtype=array_type)
581582

582583

584+
def test_periodic_boundary_mps_raises(n_sites, chi_out, array_type):
585+
"""A periodic MPS has rank 3 everywhere, so its first site looks like an MPO
586+
boundary; the interior rank gives it away.
587+
"""
588+
periodic_mps = [np.zeros((2, 2, 2), dtype=array_type)] * n_sites
589+
590+
with pytest.raises(ValueError, match="open boundary"):
591+
compress(periodic_mps, chi_out=chi_out, dtype=array_type)
592+
593+
ok_mpo = qtn.MPO_identity(n_sites, phys_dim=2, dtype=array_type).arrays
594+
with pytest.raises(ValueError, match="open boundary"):
595+
apply(periodic_mps, ok_mpo, chi_out=chi_out, dtype=array_type)
596+
597+
598+
def test_inconsistent_site_ranks_raise(n_sites, chi_out, array_type):
599+
"""Both ends can be rank 3 and still disagree with an interior site."""
600+
mpo = list(qtn.MPO_identity(n_sites, phys_dim=2, dtype=array_type).arrays)
601+
mpo[-2] = np.zeros((mpo[-2].shape[0], mpo[-2].shape[1], 2), dtype=array_type)
602+
603+
with pytest.raises(ValueError, match="open boundary"):
604+
compress(mpo, chi_out=chi_out, dtype=array_type)
605+
606+
ok_mpo = qtn.MPO_identity(n_sites, phys_dim=2, dtype=array_type).arrays
607+
with pytest.raises(ValueError, match="open boundary"):
608+
apply(ok_mpo, mpo, chi_out=chi_out, dtype=array_type)
609+
610+
611+
def test_mismatched_boundary_ranks_raise(chi_out, array_type):
612+
"""The two ends of a train must have the same rank, also for two sites."""
613+
two_site = [
614+
np.zeros((2, 2, 2), dtype=array_type),
615+
np.zeros((2, 2), dtype=array_type),
616+
]
617+
with pytest.raises(ValueError, match="open boundary"):
618+
compress(two_site, chi_out=chi_out, dtype=array_type)
619+
620+
longer = [
621+
np.zeros((2, 2, 2), dtype=array_type),
622+
np.zeros((2, 2, 2, 2), dtype=array_type),
623+
np.zeros((2, 2, 2), dtype=array_type),
624+
np.zeros((2, 2, 2, 2), dtype=array_type),
625+
]
626+
with pytest.raises(ValueError, match="open boundary"):
627+
compress(longer, chi_out=chi_out, dtype=array_type)
628+
629+
630+
# -----------------------------------------------
631+
# --- Test chi_out / cutoff argument validation --
632+
# -----------------------------------------------
633+
634+
635+
@pytest.mark.parametrize(
636+
("chi_out", "error"),
637+
[(0, ValueError), (-3, ValueError), (2.5, TypeError), (True, TypeError)],
638+
)
639+
def test_invalid_chi_out_raises(n_sites, phys_dim, chi_out, error, array_type):
640+
"""Non-positive chi_out raises ValueError, a non-integer one TypeError."""
641+
H = qtn.MPO_identity(n_sites, phys_dim=phys_dim, dtype=array_type)
642+
psi = qtn.MPS_rand_state(n_sites, bond_dim=4, phys_dim=phys_dim, dtype=array_type)
643+
644+
with pytest.raises(error, match="chi_out"):
645+
apply(H.arrays, psi.arrays, chi_out=chi_out, dtype=array_type)
646+
with pytest.raises(error, match="chi_out"):
647+
compress(H.arrays, chi_out=chi_out, dtype=array_type)
648+
649+
650+
@pytest.mark.parametrize("cutoff", [2.0, -1.0])
651+
def test_invalid_cutoff_raises(n_sites, phys_dim, chi_out, cutoff, array_type):
652+
"""cutoff outside [0.0, 1.0) must raise ValueError, not silently misbehave."""
653+
H = qtn.MPO_identity(n_sites, phys_dim=phys_dim, dtype=array_type)
654+
psi = qtn.MPS_rand_state(
655+
n_sites, bond_dim=chi_out, phys_dim=phys_dim, dtype=array_type
656+
)
657+
658+
with pytest.raises(ValueError, match="cutoff"):
659+
apply(H.arrays, psi.arrays, chi_out=chi_out, cutoff=cutoff, dtype=array_type)
660+
with pytest.raises(ValueError, match="cutoff"):
661+
compress(H.arrays, chi_out=chi_out, cutoff=cutoff, dtype=array_type)
662+
663+
664+
def test_numpy_integer_chi_out_accepted(n_sites, phys_dim, chi_out, array_type):
665+
"""chi_out as a numpy integer (np.int64 etc.) must be accepted, not rejected.
666+
667+
validate_chi_out deliberately checks `numbers.Integral` rather than
668+
`isinstance(chi_out, int)` so numpy integer types aren't rejected.
669+
"""
670+
H = qtn.MPO_identity(n_sites, phys_dim=phys_dim, dtype=array_type)
671+
psi = qtn.MPS_rand_state(
672+
n_sites, bond_dim=chi_out, phys_dim=phys_dim, dtype=array_type
673+
)
674+
675+
psi_compress = as_mps(
676+
apply(H.arrays, psi.arrays, chi_out=np.int64(chi_out), dtype=array_type)
677+
)
678+
np.testing.assert_allclose(psi.distance(psi_compress), 0.0, atol=1e-6)
679+
680+
583681
# -----------------------------------------
584682
# --- Check for performance regressions ---
585683
# -----------------------------------------

0 commit comments

Comments
Ā (0)