qiskit icon indicating copy to clipboard operation
qiskit copied to clipboard

Add Rust representation of `EquivalenceLibrary`

Open raynelfss opened this issue 1 year ago • 15 comments

Fixes #12278

Summary

As we continue our efforts to make more of the transpiler and circuit's functionality into rust. After #12459 added an infrastructure for Gates and Operations in Rust, it will be important for us to think of how to keep track of equivalent implementations of said gates that might not work in different Backends, which is why the EquivalenceLibrary exists.

The following commits add a rust representation of the EquivalenceLibrary structure.

Details and comments

Implementation:

These modifications consist of two parts:

  • A rust PyO3 class named: EquivalenceLibrary that implements most of the functionality of the python counterpart while keeping equivalent functionality in rust.
  • A python extension of the PyO3 class which is only used for the draw() method.

Modifications:

  • A couple of modifications were made to some of the defined structures under the qiskit._accelerate.circuit crate.

Possible issues:

  • There is currently no way of creating a PyDiGraph natively from rust. However, rustworkx-core allows the creation of a DiGraph that has the same functionality. If an instance of PyDiGraph is needed from Python, an alternative method called graph will assemble and store a PyDiGraph instance equivalent to the one present in the EquivalenceLibrary instance. This being only a representation should not modify the internal data of the graph stored in rust and will be discarded every time an equivalency is added or modified.
  • This implementation uses PyO3's subclass property to allow the rust PyClass to be extended from Python. However, this might lead to unexpected behavior if one were to extend the python class, as these need to be extended using the __new__ method instead of __init__. It is highly unlikely that this class would be extended further, but it is important to note this change in behavior.

Blocks

  • [ ] #12913
  • [x] #12794
  • [x] #12730
  • [x] #12766
  • [x] #12697
  • [x] #12698

Other comments:

  • Feel free to leave any reviews or comments.

raynelfss avatar Jun 14 '24 22:06 raynelfss

One or more of the following people are relevant to this code:

  • @Eric-Arellano
  • @Qiskit/terra-core
  • @kevinhartman
  • @mtreinish

qiskit-bot avatar Jun 14 '24 22:06 qiskit-bot

Nice! Did you already get to benchmark the basis translator with the oxidized equivalence library? 🙂

Cryoris avatar Jun 17 '24 09:06 Cryoris

Nice! Did you already get to benchmark the basis translator with the oxidized equivalence library? 🙂

Not yet @Cryoris, I am having some issues with the BasisTranslator, once I look at those will get to benchmarking it.

raynelfss avatar Jun 17 '24 15:06 raynelfss

About the failing test:

test.python.transpiler.test_basis_translator.TestBasisExamples.test_cx_bell_to_ecr

I'm considering changing that one failing test to use the equivalencies selected using the current methods since they seem to produce similarly equivalent results with the only difference being which circuit is selected for it, since it seems to be the only thing holding this redefinition back.

This is what the test does:

from qiskit.circuit import QuantumCircuit, QuantumRegister
from qiskit.converters import dag_to_circuit, circuit_to_dag
from qiskit.quantum_info import Operator
from qiskit.transpiler.passes.basis import BasisTranslator
from qiskit.circuit.equivalence_library import (StandardEquivalenceLibrary as std_eqlib)

pi: float = 3.14

"""Verify we can translate a CX bell to ECR,U."""
bell = QuantumCircuit(2)
bell.h(0)
bell.cx(0, 1)

in_dag = circuit_to_dag(bell)
out_dag = BasisTranslator(std_eqlib, ["ecr", "u"]).run(in_dag)

qr = QuantumRegister(2, "q")
expected = QuantumCircuit(2)
expected.u(pi / 2, 0, pi, qr[0])
expected.u(0, 0, -pi / 2, qr[0])
expected.u(pi, 0, 0, qr[0])
expected.u(pi / 2, -pi / 2, pi / 2, qr[1])
expected.ecr(0, 1)
expected_dag = circuit_to_dag(expected)

# Use assert_eq or just print the result
out_dag == expected_dag
Operator.from_circuit(bell) == Operator.from_circuit(dag_to_circuit(out_dag)

The problem happens when the BasisTranslator picks the equivalence for a CX gate. Where it would usually pick the following equivalence:

global phase: 7π/4
     ┌──────────┐┌───────┐┌──────┐
q_0: ┤ Rz(-π/2) ├┤ Ry(π) ├┤0     ├
     ├─────────┬┘└───────┘│  Ecr │
q_1: ┤ Rx(π/2) ├──────────┤1     ├
     └─────────┘          └──────┘

It ends up picking the following:

global phase: π/4
     ┌─────┐ ┌──────┐┌───┐
q_0: ┤ Sdg ├─┤0     ├┤ X ├
     ├─────┴┐│  Ecr │└───┘
q_1: ┤ √Xdg ├┤1     ├─────
     └──────┘└──────┘    

And so the produced graph is different:

Produced:

        ┌────────────┐   ┌─────────────┐┌──────┐┌──────────┐
q_0: ───┤ U(π/2,0,π) ├───┤ U(0,0,-π/2) ├┤0     ├┤ U(π,0,π) ├
     ┌──┴────────────┴──┐└─────────────┘│  Ecr │└──────────┘
q_1: ┤ U(-π/2,-π/2,π/2) ├───────────────┤1     ├────────────

Expected:

       ┌────────────────┐  ┌──────────────┐┌─────────────┐┌──────┐
q_0: ──┤ U(1.57,0,3.14) ├──┤ U(0,0,-1.57) ├┤ U(3.14,0,0) ├┤0     ├
     ┌─┴────────────────┴─┐└──────────────┘└─────────────┘│  Ecr │
q_1: ┤ U(1.57,-1.57,1.57) ├───────────────────────────────┤1     ├

I'm curious as to what the @Qiskit/terra-core team thinks about this.

raynelfss avatar Jun 22 '24 16:06 raynelfss

This looks great for the most part. I left a few questions and suggestions.

jlapeyre avatar Sep 19 '24 21:09 jlapeyre