Pyzx interface - #782
Conversation
… ruff formating on pyzx_converter.py
purva-thakre
left a comment
purva-thakre
left a comment
There was a problem hiding this comment.
Took a quick look through the changes. Will do a deeper review sometime next week.
Great work so far! CU gate from pyzx is missing. Check all gates are covered in both Qrisp and pyzx.
I do not think you have any unit tests for measurement/resets etc as well.
| Development | ||
| ----------- | ||
|
|
||
| * Added a PyZX converter |
There was a problem hiding this comment.
| * Added a PyZX converter | |
| * Added a PyZX bidirectional conversion interace |
make sure to add yourself to first time contributors at the bottom of this file.
| * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 | ||
| ******************************************************************************** | ||
| """ | ||
|
|
There was a problem hiding this comment.
I know when I originally wrote the issue to follow the approach of the cirq converter. However, this old approach leaves way too many if elif chains in both converters.
| def test_single_qubit_circuit_qrisp_to_pyzx(): | ||
| qc = _build_single_qubit_qrisp_circuit() | ||
| _test_qrisp_to_pyzx(qc) | ||
|
|
||
|
|
||
| def test_multi_qubit_circuit_qrisp_to_pyzx(): | ||
| qc = _build_multi_qubit_qrisp_circuit() | ||
| _test_qrisp_to_pyzx(qc) | ||
|
|
||
|
|
||
| def test_single_qubit_circuit_pyzx_to_qrisp(): | ||
| c = _build_single_qubit_pyzx_circuit() | ||
| _test_pyzx_to_qrisp(c) | ||
|
|
||
|
|
||
| def test_multi_qubit_circuit_pyzx_to_qrisp(): | ||
| c = _build_multi_qubit_pyzx_circuit() | ||
| _test_pyzx_to_qrisp(c) | ||
|
|
||
|
|
||
| def test_single_qubit_circuit_roundtrip(): | ||
| qc = _build_single_qubit_qrisp_circuit() | ||
| _test_roundtrip(qc) | ||
|
|
||
|
|
||
| def test_multi_qubit_circuit_roundtrip(): | ||
| qc = _build_multi_qubit_qrisp_circuit() | ||
| _test_roundtrip(qc) | ||
|
|
||
|
|
||
| def test_single_qubit_circuit_roundtrip_reverse(): | ||
| c = _build_single_qubit_pyzx_circuit() | ||
| _test_roundtrip_reverse(c) | ||
|
|
||
|
|
||
| def test_multi_qubit_circuit_roundtrip_reverse(): | ||
| c = _build_multi_qubit_pyzx_circuit() | ||
| _test_roundtrip_reverse(c) |
There was a problem hiding this comment.
add docstrings to help future maintainers understand why a particular unit test was written
| if gate_map[_gate.name] is not None: | ||
| add_gate(_gate) | ||
| else: | ||
| raise ValueError(f"{_gate.name} gate has no Qrisp equivalent and cannot be decomposed either.") |
There was a problem hiding this comment.
add a mock test to verify the converter raises an error when it runs into this.
| elif instr.op.definition: | ||
| pyzx_circuit.append(convert_to_pyzx(instr.op.definition), mask=pyxz_op_qubits) | ||
| else: | ||
| raise ValueError(f"{name} gate has no PyXZ equivalent and no definition to decompose.") |
There was a problem hiding this comment.
add a mock test to verify the converter raises an error as expected.
| from fractions import Fraction | ||
|
|
||
|
|
||
| def convert_to_pyzx(qrisp_circuit): |
There was a problem hiding this comment.
add type hints. I know the other files are missing this. But we should get into the habit of making thisngs easier when we automate type checks in the future.
| return pyzx_circuit | ||
|
|
||
|
|
||
| def convert_from_pyzx(pyzx_circuit): |
There was a problem hiding this comment.
type hints here as well
| transpiled = qrisp_circuit.transpile(transpile_predicate=_transpile_predicate) | ||
| except Exception as exc: | ||
| raise ValueError( | ||
| f"Gates {unknown} could not be transpiled and are not supported by the Qrisp to Cirq converter." |
There was a problem hiding this comment.
I htink you mean the pyzx converter here 😄 .
|
|
||
| def add_gate(gate): | ||
| # single-qubit, parameter-free gates and non-unitary operations | ||
| if gate.name in ["NOT", "Y", "Z", "HAD", "SX", "S", "T", "Measurement", "Reset"]: |
There was a problem hiding this comment.
make sure the dagger of S, T and SX are also handled correctly.
|
forgot to add: check if your function is properly covered via pytest-coverage. That workflow is still running right now. Here is a link to how you can read the generated coverage: https://pytest-cov.readthedocs.io/en/latest/reporting.html#reporting |
3 participants
Description
Implementation of a converter from Qrisp to PyZX and vice versa.
Related issues
Resolves #771
Type of Change
Breaking Change?
What was changed?
pyzx_converter.pywas added, containing two functions,convert_to_pyzxandconvert_from_pyzx.QuantumCircuitclass.