Skip to content

MPS Synthesis#7752

Open
ACE07-Sev wants to merge 34 commits intoquantumlib:mainfrom
ACE07-Sev:mps
Open

MPS Synthesis#7752
ACE07-Sev wants to merge 34 commits intoquantumlib:mainfrom
ACE07-Sev:mps

Conversation

@ACE07-Sev
Copy link
Contributor

Closes #7650 .

@ACE07-Sev ACE07-Sev requested review from a team and vtomole as code owners November 13, 2025 11:02
@ACE07-Sev ACE07-Sev requested a review from viathor November 13, 2025 11:02
@github-actions github-actions bot added the size: L 250< lines changed <1000 label Nov 13, 2025
@codecov
Copy link

codecov bot commented Nov 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.63%. Comparing base (6689231) to head (022d6c4).

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #7752    +/-   ##
========================================
  Coverage   99.63%   99.63%            
========================================
  Files        1108     1111     +3     
  Lines       99571    99713   +142     
========================================
+ Hits        99205    99347   +142     
  Misses        366      366            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ACE07-Sev
Copy link
Contributor Author

I added single qubit state prep manually, it should be removed when cirq gets state preparation added. I'll be sure to remind in the next cirq cynq.

@pavoljuhas
Copy link
Collaborator

I need to check a few things with colleagues; I will have an update early next week.

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

Please see inline comments. Otherwise LGTM, thank you for contributing this.

PS: I'd like to loop in @tanujkhattar for more comments after we converge on the initial review.

Comment on lines +39 to +40
state = np.random.rand(2**N) + 1j * np.random.rand(2**N)
state /= np.linalg.norm(state)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please reuse cirq.testing.random_superposition instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked, and random_superposition can have volume-law states which are hard to approximate with vanilla MPS (I'll add the sweeping I mentioned before soon, not in this PR though). Is it alright that for this test I keep it as is since the one I have there produces area-law states?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we can leave it as is for the initial version.

I am a bit surprised that the tested state vector seems to need coefficients with only positive real and imaginary components; the test fails if some are allowed to be negative (patch below)

Is that an expected behavior for MPS encoding?
cc @tanujkhattar

diff --git a/cirq-core/cirq/contrib/mps_synthesis/mps_sequential_test.py b/cirq-core/cirq/contrib/mps_synthesis/mps_sequential_test.py
index 2ec1b90a4..f3b1c899d 100644
--- a/cirq-core/cirq/contrib/mps_synthesis/mps_sequential_test.py
+++ b/cirq-core/cirq/contrib/mps_synthesis/mps_sequential_test.py
@@ -35 +35 @@ def test_compile_area_law_states(num_qubits: int) -> None:
-    state = np.random.rand(2**num_qubits) + 1j * np.random.rand(2**num_qubits)
+    state = 2 * np.random.rand(2**num_qubits) - 1 + 1j * np.random.rand(2**num_qubits)

…s internal module only.

- Added assertion to `_gram_schmidt` to raise an error should `matrix` not be square to catch edge cases not known at the time of changing this code.
- Changed class name to `MPSSequential`.
- Added class docstring.
- Removed `convention` from class attributes.
- Added indentation to docstring sections where needed.
- Removed `logger` uses.
- Added `mps_circuit_from_statevector` to `__init__.py` for ease of use.
- Used `cirq.testing.random_superposition` in testers where possible, except `test_compile_area_law_states` which requires strictly area-law entangled states as part of the test.
- Rewrote `test_compile_trivial_state_with_mps_pass` to avoid using qasm and avoid inter-module dependency.
- Used `np.testing.assert_allclose` for about exact matching assertions.
@ACE07-Sev
Copy link
Contributor Author

Pushed commit that addressed as many comments as I could. 4 comments remain which need your kind insight.

Main comment remaining is converging on optimal/preferred implementation for _gram_schmidt.

@ACE07-Sev
Copy link
Contributor Author

@pavoljuhas Greetings sir,

Hope you are well. Have you had a chance to review the new additions?

@pavoljuhas
Copy link
Collaborator

Have you had a chance to review the new additions?

I'll have the review in later today. Sorry about the delay.

Let us stick to the present pattern.
No change in code function.
Address missing coverage.
`_gram_schmidt` is module-local and should not be used as public API.
Per `_gram_schmidt` type annotation the matrix has True iscomplexobj.
These are already present in the function signature.
No change in the effective code.
Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

I have pushed in a couple of small clean ups which should unblock the failing coverage. Looks OK for my part, but I'd like to have another check by @tanujkhattar.

Tanuj - could you please give this a quick look?

@mhucka
Copy link
Contributor

mhucka commented Feb 10, 2026

@tanujkhattar Gentle ping …

@mhucka mhucka added triage/discuss Needs decision / discussion, bring these up during Cirq Cynque and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Feb 18, 2026
@wjhuggins
Copy link
Collaborator

I will help review.

@pavoljuhas pavoljuhas assigned wjhuggins and unassigned tanujkhattar Feb 24, 2026
@pavoljuhas
Copy link
Collaborator

@wjhuggins - gentle ping, can you PTAL?

@pavoljuhas pavoljuhas requested a review from wjhuggins March 4, 2026 19:25
@wjhuggins
Copy link
Collaborator

Sorry for the delay, I've been working on this and I'm a little confused about something but I'll put my comments below anyway.

Besides the below comments, I am surprised by how bad the method is when applied to MPS states with relatively low bond dimension. I implemented a quick and dirty version of the test described in 3. and while the method worked perfectly with bond dimension two it failed pretty badly with bond dimension >= 3. I'm not sure if this indicates a bug or some failing of the method but I don't think we should include this in Cirq unless we can get it working somewhat well. (It's possible I messed something up, but implementing the test from 3. is a good idea anyway I think).

Note that Gemini and I collaborated on the notes below but I looked them over pretty carefully.

1. generate_layer assumptions

The generate_layer function in mps_sequential.py currently assumes it will only be called with an MPS compressed to a maximum bond dimension of 2 (max_bond=2). If passed an MPS with a larger bond dimension, it will fail due to:

  • Creating non-power-of-2 matrices (e.g., $6 \times 6$ for bond dim 3) which are invalid quantum gates.
  • Shape mismatches during isometry embedding when bond dimensions grow quickly ($2 \cdot d_{left} &lt; d_{right}$).

Action Item: Either modify generate_layer to handle arbitrary bond dimensions (by padding to nearest powers of 2), or clearly document this restriction in the function's docstring and perhaps add an assertion to enforce max_bond <= 2.

2. Early Unitarity Correction and Verification

At the very end of the algorithm, an SVD is used to correct floating-point errors in the matrices before appending them to the circuit:

U, s, Vh = np.linalg.svd(unitary)
unitary = U @ Vh

Do we really need this if we are getting the gates from a process that should itself be unitary up to floating point errors? If so, I would still propose the changes below:

Proposed Changes:

  1. Move this correction earlier: Move this step inside generate_layer (or right after it is called) so the matrices are strictly unitary before their inverses are applied to disentangled_mps. This ensures the algorithm simulates the exact operations that will eventually run on hardware, preventing numerical drift from compounding during the disentanglement sweep.
  2. Verify Unitarity: If the matrix was already close to unitary, its singular values (s) must be very close to 1.0. Applying U @ Vh silently could hide a bug so if it's necessary let's add an assertion to catch them early:
assert np.allclose(s, 1.0, atol=1e-5), "Matrix drifted significantly from unitary"

3. test_compile_area_law_states produces volume-law states

The test_compile_area_law_states function currently attempts to generate an area-law entangled state using np.random.rand(2**N). This inherently produces a highly-entangled volume-law state.

Action Item:
Use quimb.tensor.MPS_rand_state to generate authentic random area-law states by specifying a small bond dimension. It is recommended to parameterize this test over system size, initial bond dimension, and whether the state is real or complex to ensure broad coverage. We should ensure that the method words exactly (up to numerical precision) for bond dimension 2 and that it works reliably when the bond dimension is still low but larger than 2.

(Note: Testing purely real states will surface a NumPy type-casting bug in generate_layer. To fix it, simply change the matrix initialization to explicitly use dtype=np.complex128 so that Gram-Schmidt does not try to write complex projections into a float64 view).

import quimb.tensor as qtn

@pytest.mark.parametrize("num_qubits", [5, 8, 10])
@pytest.mark.parametrize("bond_dim", [2, 5, 20])
@pytest.mark.parametrize("is_complex", [True, False])
def test_compile_area_law_states(num_qubits: int, bond_dim: int, is_complex: bool) -> None:
    np.random.seed(42)
    dtype = complex if is_complex else float
    
    # Generate an authentic area-law state via MPS
    mps = qtn.MPS_rand_state(num_qubits, bond_dim=bond_dim, dtype=dtype)
    state = mps.to_dense()
    state /= np.linalg.norm(state)
    
    # ... synthesis and fidelity assertions ...

4. Memory Scalability and API Design

The primary advantage of MPS synthesis is compiling circuits for weakly entangled states that are too large to represent as dense vectors (e.g., $N &gt; 30$). However, the current API strictly requires a dense statevector as input, which severely limits its scalability.

Action Item: The top-level API (mps_circuit_from_statevector or a new sister function) should optionally accept a qtn.MatrixProductState directly. This allows users to compile circuits for states derived from tensor network algorithms (like DMRG) without triggering Out-Of-Memory errors.

5. $O(2^N)$ Bottleneck in the Fidelity Check

Inside the mps_to_circuit_approx loop, the algorithm checks if it can stop early by measuring fidelity against the zero state:

fidelity = np.abs(np.vdot(disentangled_mps.to_dense(), zero_state)) ** 2

Calling .to_dense() forces the allocation of the full $2^N$ statevector. If the API is updated to support large MPS inputs (as suggested above), this line will immediately crash.

Action Item: The fidelity between any state and the $|00\dots0\rangle$ state is simply the squared magnitude of the all-zeros amplitude. This can be computed efficiently in $O(N)$ time by creating a trivial zero-state MPS and contracting the two tensor networks, or by simply evaluating the tensors at index 0.

6. test_compile_trivial_state_with_mps_pass Setup

In the mps_sequential_test.py file, the test for a trivial (product) state constructs a complex 10-qubit circuit with CNOTs just to immediately uncompute them and yield a product state.

Action Item: This setup is more complex than it needs to be and hides the intent of the test. A trivial product state can be constructed much more directly either by generating an MPS with bond_dim=1 using quimb, or by generating a series of random 1-qubit states and joining them using np.kron.

7. Non-deterministic Compilation (Minor)

Inside the _gram_schmidt function, linearly dependent or zero columns are replaced using an unseeded random number generator (np.random.uniform). This makes the compiler non-deterministic—calling synthesis twice on the exact same state may yield different unitary matrices (with arbitrary rotations in the null spaces).

Action Item: Ensure compilation is strictly deterministic by either accepting a random seed, using a local seeded np.random.RandomState, or using a deterministic basis completion algorithm. This should apply to the tests too. Really anywhere we use randomness.

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

Per feedback from wjhuggins this is not yet working well enough to be included; for details, please see #7752 (comment).

Can you update the PR to address these comments?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: L 250< lines changed <1000 triage/discuss Needs decision / discussion, bring these up during Cirq Cynque

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide features for MPS state preparation

5 participants