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.

Sign up for GitHub

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

[bugfix] Default value of chi causes an error when state is copied #93

Merged
merged 2 commits into from
Apr 3, 2024
Merged

[bugfix] Default value of chi causes an error when state is copied #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6aa4d1
Now checking if chi is set to the default 'essentially unbounded' val…
PabloAndresCQ Apr 3, 2024
e82322d
Adding a test of copy state using different configurations
PabloAndresCQ Apr 3, 2024
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
4 changes: 3 additions & 1 deletion pytket/extensions/cutensornet/structured_state/general.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,16 @@ def __init__(
ValueError: If the value of ``chi`` is set below 2.
ValueError: If the value of ``truncation_fidelity`` is not in [0,1].
"""
_CHI_LIMIT = 2**60
if (
chi is not None
and chi < _CHI_LIMIT
Copy link
Collaborator

Choose a reason for hiding this comment

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

You mean <= ?
And are you usually adding tests for bugfixes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope, it should be <. If it's equal to the "default value" _CHI_LIMIT, then this condition must evaluate to false, so that the error is not raised.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, we usually add test for bugfixes when relevant (and easy). I'm happy to add one for this, since it satisfies both requirements :P

and truncation_fidelity is not None
and truncation_fidelity != 1.0
):
raise ValueError("Cannot fix both chi and truncation_fidelity.")
if chi is None:
chi = 2**60 # In practice, this is like having it be unbounded
chi = _CHI_LIMIT # In practice, this is like having it be unbounded
if truncation_fidelity is None:
truncation_fidelity = 1

Expand Down
38 changes: 38 additions & 0 deletions tests/test_structured_state.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,44 @@ def test_init() -> None:
assert ttn_gate.is_valid()


@pytest.mark.parametrize(
"algorithm",
[
SimulationAlgorithm.MPSxGate,
SimulationAlgorithm.MPSxMPO,
SimulationAlgorithm.TTNxGate,
],
)
def test_copy(algorithm: SimulationAlgorithm) -> None:
simple_circ = Circuit(2).H(0).H(1).CX(0, 1)

with CuTensorNetHandle() as libhandle:

# Default config
cfg = Config()
state = simulate(libhandle, simple_circ, algorithm, cfg)
assert state.is_valid()
copy_state = state.copy()
assert copy_state.is_valid()
assert np.isclose(copy_state.vdot(state), 1.0, atol=cfg._atol)

# Bounded chi
cfg = Config(chi=8)
state = simulate(libhandle, simple_circ, algorithm, cfg)
assert state.is_valid()
copy_state = state.copy()
assert copy_state.is_valid()
assert np.isclose(copy_state.vdot(state), 1.0, atol=cfg._atol)

# Bounded truncation_fidelity
cfg = Config(truncation_fidelity=0.9999)
state = simulate(libhandle, simple_circ, algorithm, cfg)
assert state.is_valid()
copy_state = state.copy()
assert copy_state.is_valid()
assert np.isclose(copy_state.vdot(state), 1.0, atol=cfg._atol)


def test_canonicalise_mps() -> None:
cp.random.seed(1)
circ = Circuit(5)
Expand Down