Finalize Rust representation for standard gates #12709

ElePT · 2024-07-02T13:28:45Z

Summary

Added missing gate definitions and update rust equivalence tests accordingly.
Reordered gates in ascending number of qubits and alphabetical-ish order
Reordered gate definitions and matrices to follow enum order for consistency
Removed C4XGate from list of standard gates in rust

Details and comments

This PR completes #12566 🎉

The proposed gate order is as follows (52 gates in total). Starting 1q gate list from HGate as suggested by @alexanderivrii.

pub enum StandardGate {
    // 0-qubits
    GlobalPhaseGate = 0,
    // 1-qubit
    HGate = 1,
    IGate = 2,
    XGate = 3,
    YGate = 4,
    ZGate = 5,
    PhaseGate = 6,
    RGate = 7,
    RXGate = 8,
    RYGate = 9,
    RZGate = 10,
    SGate = 11,
    SdgGate = 12,
    SXGate = 13,
    SXdgGate = 14,
    TGate = 15,
    TdgGate = 16,
    UGate = 17,
    U1Gate = 18,
    U2Gate = 19,
    U3Gate = 20,
    // 2-qubits
    CHGate = 21,
    CXGate = 22,
    CYGate = 23,
    CZGate = 24,
    DCXGate = 25,
    ECRGate = 26,
    SwapGate = 27,
    ISwapGate = 28,
    CPhaseGate = 29,
    CRXGate = 30,
    CRYGate = 31,
    CRZGate = 32,
    CSGate = 33,
    CSdgGate = 34,
    CSXGate = 35,
    CUGate = 36,
    CU1Gate = 37,
    CU3Gate = 38,
    RXXGate = 39,
    RYYGate = 40,
    RZZGate = 41,
    RZXGate = 42,
    XXMinusYYGate = 43,
    XXPlusYYGate = 44,
    // 3-qubits
    CCXGate = 45,
    CCZGate = 46,
    CSwapGate = 47,
    RCCXGate = 48,
    // 4-qubits
    C3XGate = 49,
    C3SXGate = 50,
    RC3XGate = 51,
}

jakelishman · 2024-07-02T13:41:02Z

crates/circuit/src/circuit_instruction.rs

+        // The default ctrl_states are 1, "1" and None. These are the only cases where controlled
+        // gates can be standard.
+        let is_default_ctrl_state: bool = match py_op.getattr(py, intern!(py, "ctrl_state")) {
+            Ok(c_state) => match c_state.extract::<Option<i32>>(py) {
+                Ok(c_state_int) => match c_state_int {
+                    Some(c_int) => c_int == 1,
+                    None => true,
+                },
+                Err(_) => false,
+            },
+            Err(_) => false,
+        };


CCX's default control state is 3 - did we not include that in the standard gates?

Aha, that makes sense, I wasn't thinking beyond 2q gates. I will generalize the logic.

Addressed in #12659: 05fe760 05fe760

coveralls · 2024-07-02T13:53:50Z

Pull Request Test Coverage Report for Build 9761907886

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

527 of 565 (93.27%) changed or added relevant lines in 9 files are covered.
16 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.006%) to 89.81%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/circuit/src/circuit_instruction.rs	8	10	80.0%
qiskit/circuit/quantumcircuit.py	12	16	75.0%
crates/circuit/src/operations.rs	399	431	92.58%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	4	92.11%
crates/qasm2/src/parse.rs	12	96.69%

Totals
Change from base Build 9757893531:	0.006%
Covered Lines:	64911
Relevant Lines:	72276

💛 - Coveralls

coveralls · 2024-07-09T08:16:41Z

Pull Request Test Coverage Report for Build 9856279167

Details

599 of 624 (95.99%) changed or added relevant lines in 2 files are covered.
19 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.04%) to 89.907%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/circuit/src/operations.rs	560	585	95.73%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	94.02%
crates/qasm2/src/lex.rs	1	93.38%
crates/circuit/src/operations.rs	17	85.29%

Totals
Change from base Build 9855880439:	0.04%
Covered Lines:	65706
Relevant Lines:	73082

💛 - Coveralls

…order. Make definitions and matrices consistent with new gate order. Remove C4XGate (second mcx) from list.

qiskit-bot · 2024-07-09T11:25:43Z

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

@Qiskit/terra-core
@kevinhartman
@mtreinish

…gates-6

mtreinish

This LGTM, I personally probably wouldn't have bothered to reorder the match statements for the matrices and definitions because the order doesn't matter so much there. But being consistent with the canonical order makes sense.

* Add missing gate definitions * Reorder gates, following number of qubits and a sort of alphabetical order. Make definitions and matrices consistent with new gate order. Remove C4XGate (second mcx) from list.

jakelishman reviewed Jul 2, 2024

View reviewed changes

mtreinish linked an issue Jul 2, 2024 that may be closed by this pull request

Add remaining standard gates to rust representation #12566

Closed

1ucian0 added the Rust This PR or issue is related to Rust code in the repository label Jul 3, 2024

Add missing gate definitions

661feb7

ElePT force-pushed the oxidize-gates-6 branch from ef6be85 to 661feb7 Compare July 9, 2024 07:51

Reorder gates, following number of qubits and a sort of alphabetical …

1f7dc0c

…order. Make definitions and matrices consistent with new gate order. Remove C4XGate (second mcx) from list.

ElePT marked this pull request as ready for review July 9, 2024 11:25

ElePT requested a review from a team as a code owner July 9, 2024 11:25

Merge branch 'main' of https://github.com/Qiskit/qiskit into oxidize-…

91af88b

…gates-6

ElePT added Changelog: None Do not include in changelog mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library labels Jul 9, 2024

mtreinish approved these changes Jul 12, 2024

View reviewed changes

mtreinish added this pull request to the merge queue Jul 12, 2024

Merged via the queue into Qiskit:main with commit 3e2a6e8 Jul 12, 2024
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Finalize Rust representation for standard gates #12709

Finalize Rust representation for standard gates #12709

ElePT commented Jul 2, 2024 •

edited

Loading

jakelishman Jul 2, 2024

ElePT Jul 2, 2024

ElePT Jul 2, 2024

coveralls commented Jul 2, 2024 •

edited

Loading

coveralls commented Jul 9, 2024 •

edited

Loading

qiskit-bot commented Jul 9, 2024

mtreinish left a comment

Finalize Rust representation for standard gates #12709

Finalize Rust representation for standard gates #12709

Conversation

ElePT commented Jul 2, 2024 • edited Loading

Summary

Details and comments

jakelishman Jul 2, 2024

Choose a reason for hiding this comment

ElePT Jul 2, 2024

Choose a reason for hiding this comment

ElePT Jul 2, 2024

Choose a reason for hiding this comment

coveralls commented Jul 2, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9761907886

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

coveralls commented Jul 9, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9856279167

Details

💛 - Coveralls

qiskit-bot commented Jul 9, 2024

mtreinish left a comment

Choose a reason for hiding this comment

ElePT commented Jul 2, 2024 •

edited

Loading

coveralls commented Jul 2, 2024 •

edited

Loading

coveralls commented Jul 9, 2024 •

edited

Loading