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

Jump to bottom

Implement RGate in Rust #12662

Merged
merged 7 commits into from
Jun 27, 2024
Merged

Implement RGate in Rust #12662

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2a064bd
Implement RGate in Rust
jlapeyre Jun 26, 2024
a034be5
Update crates/circuit/src/operations.rs
jlapeyre Jun 26, 2024
7eef0f3
Fix error in decomposition of RGate
jlapeyre Jun 26, 2024
49c8dcd
Factor cloning the Param enum in Rust
jlapeyre Jun 26, 2024
043133f
Run cargo fmt
jlapeyre Jun 26, 2024
07fec37
Implement and use addition for enum Param
jlapeyre Jun 26, 2024
bff9fef
Merge branch 'main' into implement-r-gate
jlapeyre Jun 26, 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
13 changes: 13 additions & 0 deletions crates/circuit/src/gate_matrix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ const fn c64(re: f64, im: f64) -> Complex64 {
pub static ONE_QUBIT_IDENTITY: [[Complex64; 2]; 2] =
[[c64(1., 0.), c64(0., 0.)], [c64(0., 0.), c64(1., 0.)]];

#[inline]
pub fn r_gate(theta: f64, phi: f64) -> [[Complex64; 2]; 2] {
let half_theta = theta / 2.;
let cost = c64(half_theta.cos(), 0.);
let sint = half_theta.sin();
let cosphi = phi.cos();
let sinphi = phi.sin();
[
[cost, c64(-sint * sinphi, -sint * cosphi)],
[c64(sint * sinphi, -sint * cosphi), cost],
]
}

#[inline]
pub fn rx_gate(theta: f64) -> [[Complex64; 2]; 2] {
let half_theta = theta / 2.;
Expand Down
2 changes: 1 addition & 1 deletion crates/circuit/src/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ static STDGATE_IMPORT_PATHS: [[&str; 2]; STANDARD_GATE_SIZE] = [
// CRZGate = 31
["placeholder", "placeholder"],
// RGate 32
["placeholder", "placeholder"],
["qiskit.circuit.library.standard_gates.r", "RGate"],
// CHGate = 33
["qiskit.circuit.library.standard_gates.h", "CHGate"],
// CPhaseGate = 34
Expand Down
47 changes: 43 additions & 4 deletions crates/circuit/src/operations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ static STANDARD_GATE_NUM_QUBITS: [u32; STANDARD_GATE_SIZE] = [
1, 1, 1, 2, 2, 2, 3, 1, 1, 1, // 0-9
2, 2, 1, 0, 1, 1, 1, 1, 1, 1, // 10-19
1, 1, 1, 2, 2, 2, 1, 1, 1, 34, // 20-29
34, 34, 34, 2, 2, 2, 2, 2, 3, 2, // 30-39
34, 34, 1, 2, 2, 2, 2, 2, 3, 2, // 30-39
2, 2, 34, 34, 34, 34, 34, 34, 34, 34, // 40-49
34, 34, 34, // 50-52
];
Expand All @@ -249,7 +249,7 @@ static STANDARD_GATE_NUM_PARAMS: [u32; STANDARD_GATE_SIZE] = [
0, 0, 0, 0, 0, 0, 0, 1, 1, 1, // 0-9
0, 0, 0, 1, 0, 0, 1, 3, 0, 0, // 10-19
0, 0, 0, 0, 2, 2, 1, 2, 3, 34, // 20-29
34, 34, 34, 0, 1, 0, 0, 0, 0, 3, // 30-39
34, 34, 2, 0, 1, 0, 0, 0, 0, 3, // 30-39
1, 3, 34, 34, 34, 34, 34, 34, 34, 34, // 40-49
34, 34, 34, // 50-52
];
Expand Down Expand Up @@ -514,7 +514,12 @@ impl Operation for StandardGate {
_ => None,
},
Self::CRXGate | Self::CRYGate | Self::CRZGate => todo!(),
Self::RGate => todo!(),
Self::RGate => match params {
[Param::Float(theta), Param::Float(phi)] => {
Some(aview2(&gate_matrix::r_gate(*theta, *phi)).to_owned())
}
_ => None,
},
Self::CHGate => todo!(),
Self::CPhaseGate => todo!(),
Self::CSGate => todo!(),
Expand Down Expand Up @@ -954,7 +959,41 @@ impl Operation for StandardGate {
)
}),
Self::CRXGate | Self::CRYGate | Self::CRZGate => todo!(),
Self::RGate => todo!(),
Self::RGate => Python::with_gil(|py| -> Option<CircuitData> {
let theta_expr = match &params[0] {
Param::Float(theta) => Param::Float(*theta),
Param::ParameterExpression(theta) => {
Param::ParameterExpression(theta.clone_ref(py))
}
Param::Obj(_) => unreachable!(),
};
let (phi_expr1, phi_expr2) = match &params[0] {
jlapeyre marked this conversation as resolved.
Show resolved Hide resolved
Param::Float(phi) => (Param::Float(*phi - 1.0), Param::Float(-*phi + 1.0)),
Param::ParameterExpression(phi) => {
let phiexpr1 = phi
.call_method1(py, intern!(py, "__add__"), ((-PI / 2.0),))
.expect("Unexpected Qiskit python bug");
let phiexpr2 = phiexpr1
.call_method1(py, intern!(py, "__rmul__"), (-1.0,))
.expect("Unexpected Qiskit python bug");
(
Param::ParameterExpression(phiexpr1),
Param::ParameterExpression(phiexpr2),
)
}
Param::Obj(_) => unreachable!(),
};
let defparams = smallvec![theta_expr, phi_expr1, phi_expr2];
Some(
CircuitData::from_standard_gates(
py,
1,
[(Self::UGate, defparams, smallvec![Qubit(0)])],
FLOAT_ZERO,
)
.expect("Unexpected Qiskit python bug"),
)
}),
Copy link
Contributor

@ElePT ElePT Jun 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think this could be more readable if we used helper functions like multiply_param, and maybe add one for adding a scalar to a parameter in the same fashion. But I am fine merging the implementation as-is and beautifying it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My first iteration did not fit with the helper function, nor one for add. But it's changed a lot since I last tried. I'll revisit it; readability and encapsulation is important.

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 did something similar to your suggest in 49c8dcd (#12662). This factors out cloning a Param. This is only used once now, but can be used whenever a Param needs to be cloned. I find the code much more readable at the call point.

Regarding factoring out add_param: There are pros and cons to doing this. Disadvantages: The branch with -*phi + PI / 2.0 is more readable if it is not nested calls. In fact, this made it slightly easier for me to spot the error -*phi + 1.0.

Another one is that it requires two match statements rather than one, so -*phi + PI / 2.0 won't be optimized. I'm not sure, but I doubt this will matter in the future for performance.

But, I'm on the fence. Currently, the second match arm would be much more readable with your suggestion. I can implement and use add_param quickly if you prefer. I'm inclined to do it just to look at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented your suggestion in 07fec37 (#12662). I'm not 100% sure there isn't a non-negligible efficiency hit. But I think it's easier to read and will be easier to build on.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see your point, I think we could merge the code as-is and once all Rust standard gates are in place, maybe run the benchmarks with/without add_param to see if there is indeed a significant performance hit.

Self::CHGate => todo!(),
Self::CPhaseGate => todo!(),
Self::CSGate => todo!(),
Expand Down
3 changes: 3 additions & 0 deletions qiskit/circuit/library/standard_gates/r.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from qiskit.circuit.gate import Gate
from qiskit.circuit.quantumregister import QuantumRegister
from qiskit.circuit.parameterexpression import ParameterValueType
from qiskit._accelerate.circuit import StandardGate


class RGate(Gate):
Expand Down Expand Up @@ -49,6 +50,8 @@ class RGate(Gate):
\end{pmatrix}
"""

_standard_gate = StandardGate.RGate

def __init__(
self,
theta: ParameterValueType,
Expand Down
4 changes: 1 addition & 3 deletions qiskit/circuit/quantumcircuit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4649,9 +4649,7 @@ def r(
Returns:
A handle to the instructions created.
"""
from .library.standard_gates.r import RGate

return self.append(RGate(theta, phi), [qubit], [], copy=False)
return self._append_standard_gate(StandardGate.RGate, [theta, phi], qargs=[qubit])

def rv(
self,
Expand Down
2 changes: 1 addition & 1 deletion test/python/circuit/test_rust_equivalence.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from qiskit.circuit import QuantumCircuit
from qiskit.circuit.library.standard_gates import get_standard_gate_name_mapping

SKIP_LIST = {"rx", "ry", "ecr"}
SKIP_LIST = {"rx", "ry", "ecr", "r"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add the r gate to the SKIP_LIST? All others haven't been implemented yet, but r has an implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I added and removed several times while debugging. I'll recheck.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7eef0f3 (#12662). It failed because the decomposition had an error.

CUSTOM_MAPPING = {"x", "rz"}


Expand Down
Loading