Skip to content

Replacing Graphix transpiler with graphix-jcz-transpiler#484

Open
emlynsg wants to merge 63 commits into
masterfrom
add-jcz
Open

Replacing Graphix transpiler with graphix-jcz-transpiler#484
emlynsg wants to merge 63 commits into
masterfrom
add-jcz

Conversation

@emlynsg

@emlynsg emlynsg commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

This PR takes the causal flow-based transpilation method detailed in the graphix-jcz-transpiler and adds it to Graphix, replacing the previous transpilation approach into one that is more principled and consistent with the literature [1].

Major changes include:

  • Full rework of the transpiler.py module to implement $J,, CZ$ gate decomposition and use the causal-flow-based approach to construct patterns
  • Addition of a $J(\alpha)$ gate to circuits, with equivalent additions to random circuits, ops and management to codebase components related to OpenQASM
  • Updates to many tests to account for the new transpiler generating BlochMeasurement commands which need to be inferred as Pauli (usually for preprocessing)

[1] The Measurement Calculus
Vincent Danos, Elham Kashefi, Prakash Panangaden

thierry-martinez added a commit to thierry-martinez/graphix that referenced this pull request Apr 17, 2026
There is currently a bug when using Mypy to type-check code using the
latest Qiskit release (2.4.0, released on 2026-04-16).

See python/mypy#21263

This bug makes the CI `typecheck` workflow fails systematically.
See for instance:
- PR TeamGraphix#484:
  https://github.com/TeamGraphix/graphix/actions/runs/24565948645/job/71826055256
- PR TeamGraphix#481 (I pushed a commit on that branch to fix the problem afterwards):
  https://github.com/TeamGraphix/graphix/actions/runs/24528928441/job/71707158586

The temporary fix proposed in this commit is to add an upperbound in
`requirements-dev.txt`, `qiskit<2.4`, while the bug is not fixed.
thierry-martinez added a commit that referenced this pull request Apr 17, 2026
There is currently a bug when using Mypy to type-check code using the
latest Qiskit release (2.4.0, released on 2026-04-16).

See python/mypy#21263

This bug makes the CI `typecheck` workflow fails systematically.
See for instance:
- PR #484:
  https://github.com/TeamGraphix/graphix/actions/runs/24565948645/job/71826055256
- PR #481 (I pushed a commit on that branch to fix the problem afterwards):
  https://github.com/TeamGraphix/graphix/actions/runs/24528928441/job/71707158586

The temporary fix proposed in this commit is to add an upperbound in
`requirements-dev.txt`, `qiskit<2.4`, while the bug is not fixed.
Emlyn Graham and others added 6 commits April 17, 2026 18:21
* Bump the python-packages group with 2 updates

Updates the requirements on [ruff](https://github.com/astral-sh/ruff) and [qiskit](https://github.com/Qiskit/qiskit) to permit the latest version.

Updates `ruff` from 0.15.10 to 0.15.11
- [Release notes](https://github.com/astral-sh/ruff/releases)
- [Changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md)
- [Commits](astral-sh/ruff@0.15.10...0.15.11)

Updates `qiskit` to 2.4.0
- [Release notes](https://github.com/Qiskit/qiskit/releases)
- [Changelog](https://github.com/Qiskit/qiskit/blob/main/docs/release_notes.rst)
- [Commits](Qiskit/qiskit@1.0.0...2.4.0)

---
updated-dependencies:
- dependency-name: ruff
  dependency-version: 0.15.11
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: python-packages
- dependency-name: qiskit
  dependency-version: 2.4.0
  dependency-type: direct:development
  dependency-group: python-packages
...

Signed-off-by: dependabot[bot] <support@github.com>

* reverting bump to qiskit max version

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Emlyn Graham <egraham@PTB-02009984.paris.inria.fr>
Mypy currently fails when type-checking code against the latest Qiskit
release (2.4.0; see #485 and python/mypy#21263).

While #485 added an upperbound `qiskit<2.4` to `requirements-dev.txt`,
it did not configure Dependabot to respect this limit, causing PR
reverts (see commit bd9c491 in #486).

This commit adds the necessary configuration to prevent those updates.
@emlynsg emlynsg marked this pull request as ready for review April 20, 2026 14:49
@thierry-martinez

Copy link
Copy Markdown
Collaborator

Veriphix tests are likely being killed by the OOM killer. When running the Veriphix test suite locally, I observed that it hangs on the very first test of test_client.py. This test runs a delegated random circuit that only standardizes the pattern, without using Pauli presimulation or space minimization. There is a difficulty that I don't know how to address here: the new transpiler cannot match the performance of the original one without Pauli presimulation, but we do not yet know how to implement Pauli presimulation without setting the input.

@emlynsg

emlynsg commented Apr 21, 2026

Copy link
Copy Markdown
Contributor Author

Veriphix tests are likely being killed by the OOM killer. When running the Veriphix test suite locally, I observed that it hangs on the very first test of test_client.py. This test runs a delegated random circuit that only standardizes the pattern, without using Pauli presimulation or space minimization. There is a difficulty that I don't know how to address here: the new transpiler cannot match the performance of the original one without Pauli presimulation, but we do not yet know how to implement Pauli presimulation without setting the input.

I agree that seems to be the case. I can imagine it should work if either minimize_space or to_space_minimal_pattern are applied -- let's discuss tomorrow morning.

@codecov

codecov Bot commented Apr 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.73%. Comparing base (8bf1ebe) to head (e067d7a).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
graphix/transpiler.py 95.96% 9 Missing ⚠️
graphix/qasm3_exporter.py 72.72% 3 Missing ⚠️
graphix/sim/density_matrix.py 50.00% 1 Missing ⚠️
graphix/sim/statevec.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #484      +/-   ##
==========================================
- Coverage   88.85%   88.73%   -0.12%     
==========================================
  Files          49       49              
  Lines        7135     7202      +67     
==========================================
+ Hits         6340     6391      +51     
- Misses        795      811      +16     

☔ View full report in Codecov by Harness.
📢 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.

@thierry-martinez thierry-martinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! Thank you very much for this big change.

@matulni matulni left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @emlynsg for this nice work! Unfortunately I don't know the details about the JCZ transpiler so I only made comments about the code.

On a general note, your decompose_... functions lend themselves to return iterators (check the old _transpile_rzz), which are evaluated lazily and avoid allocating lists in the intermediate steps (you may also want to check map). I didn't check, so I won't make any claims on its performance (my guess is that it's not significant for the circuit sizes that we deal with). I will not bother you with it, we can leave it as is 😬

Comment thread graphix/transpiler.py Outdated
Comment thread graphix/transpiler.py Outdated
Comment thread graphix/transpiler.py Outdated
Comment thread graphix/transpiler.py Outdated
Comment thread graphix/transpiler.py Outdated
Comment thread graphix/transpiler.py Outdated
Comment thread graphix/transpiler.py Outdated
Comment thread graphix/qasm3_exporter.py Outdated
Comment thread tests/test_transpiler.py
Comment thread tests/test_transpiler.py Outdated
emlynsg and others added 4 commits April 30, 2026 14:02
Co-authored-by: matulni <m.uldemolins@gmail.com>
Co-authored-by: matulni <m.uldemolins@gmail.com>
Co-authored-by: matulni <m.uldemolins@gmail.com>

@matulni matulni left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you, very nice work. We should be able recycle some of your ideas in TranspileSwapResult in a more general API for Circuit !

I made some minor comments (and there remain a few comments from a previous review I made to resolve).

Comment thread graphix/qasm3_exporter.py
"""
if transpile:
circuit = circuit.transpile_j_to_rzh().transpile_measurements_to_z_axis()
if any(instr.kind == InstructionKind.J for instr in circuit.instruction):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we need this check, we are already checking in instruction_to_qasm3.
By the same logic, we should also check that there are not non-Z measurements (which we do at instruction_to_qasm3).

If we decide to keep it:

Suggested change
if any(instr.kind == InstructionKind.J for instr in circuit.instruction):
elif any(instr.kind == InstructionKind.J for instr in circuit.instruction):

Comment thread graphix/qasm3_exporter.py
Comment on lines +33 to +34
if transpile:
circuit = circuit.transpile_j_to_rzh().transpile_measurements_to_z_axis()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We are transpiling j and measurements in circuit_to_qasm_lines, so I think there's no need to transpile twice.

Suggested change
if transpile:
circuit = circuit.transpile_j_to_rzh().transpile_measurements_to_z_axis()

Comment thread graphix/qasm3_exporter.py
if transpile:
circuit = circuit.transpile_j_to_rzh().transpile_measurements_to_z_axis()
if any(instr.kind == InstructionKind.J for instr in circuit.instruction):
raise ValueError("J gates must be decomposed before QASM3 export using `Circuit.transpile_j_to_rzh`.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise ValueError("J gates must be decomposed before QASM3 export using `Circuit.transpile_j_to_rzh`.")
raise ValueError("J gates must be decomposed before QASM3 export using `Circuit.transpile_j_to_rzh`, or setting `transpile=True`.")

Comment thread tests/test_pattern.py

for p in [p_ref, p_test]:
p.remove_pauli_measurements()
p_test = p_test.infer_pauli_measurements()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Done below

Suggested change
p_test = p_test.infer_pauli_measurements()

Comment thread graphix/transpiler.py
pattern: Pattern
classical_outputs: tuple[int, ...]
result: _R
classical_outputs: _CO

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe it's an overkill, but do you think it would be worth to introduce a property classical_outputs to narrow the type _CO depending on the type of _R?

Comment thread graphix/transpiler.py
Comment on lines +991 to +994
def swap_statevec(self, statevec: Statevec) -> Statevec:
"""Reorder the elements of a statevector obtained from a swapped circuit to restore the qubit ordering of the original circuit."""
psi = np.transpose(statevec.psi, self.extract_output_node_indices())
return Statevec(psi.flatten(), statevec.nqubit)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the logic in this method deserves to be a method of Statevec (or even DenseState).
The reason is twofold:

  • It relies on the internal representation of statevec.psi (in fact, it will fail with the new jit backend).
  • We could use it in DenseStateBackend.sort_qubits instead of repeated calls to swap (outside of this PR).

thierry-martinez and others added 3 commits June 11, 2026 22:48
Co-authored-by: matulni <m.uldemolins@gmail.com>
Co-authored-by: matulni <m.uldemolins@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants