Skip to content

Add rust representation for SGates, TGates, and iSwap gate #12598

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
Merged
merged 4 commits into from
Jun 18, 2024
Merged

Add rust representation for SGates, TGates, and iSwap gate #12598

merged 4 commits into from
Jun 18, 2024

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Jun 18, 2024 •
edited
Loading

Summary

This PR adds some of the remaining gates from #12566. The changes are related to #12572, which adds u1, u2 and u3, and #12507, which adds the RGate.

I am tempted to modify the order of the gate table to something a bit nicer but with so many open parallel PRs that modify it I think it would be a source of bugs, and it might make more sense to leave it for later.

Details and comments

@ElePT ElePT mentioned this pull request Jun 18, 2024
Closed
@ElePT ElePT changed the title Add SGates, TGates, and iSwap gate to rust representation Add rust representation for SGates, TGates, and iSwap gate Jun 18, 2024
@mtreinish mtreinish mentioned this pull request Jun 18, 2024
Merged
@ElePT ElePT added performance Rust This PR or issue is related to Rust code in the repository Changelog: None Do not include in changelog labels Jun 18, 2024
@ElePT
Add SGate, SdgGate, iSWAP, TGate, and TdgGate to standard gates in rust.
17c84c4 
Add missing gate definitions that depended on these gates (marked as todo).
@ElePT ElePT marked this pull request as ready for review June 18, 2024 12:56
@ElePT ElePT requested a review from a team as a code owner June 18, 2024 12:56
@qiskit-bot
Copy link
Collaborator

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

  • @Cryoris
  • @Qiskit/terra-core
  • @ajavadia
  • @kevinhartman
  • @mtreinish

mtreinish
mtreinish reviewed Jun 18, 2024
View reviewed changes
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

The other thing you probably want to update here is adding the fast path for rust gates to the circuit methods. For example:

qiskit/qiskit/circuit/quantumcircuit.py

Lines 4481 to 4492 in 6d6dce3

def h(self, qubit: QubitSpecifier) -> InstructionSet:
"""Apply :class:`~qiskit.circuit.library.HGate`.
For the full matrix form of this gate, see the underlying gate documentation.
Args:
qubit: The qubit(s) to apply the gate to.
Returns:
A handle to the instructions created.
"""
return self._append_standard_gate(StandardGate.HGate, [], qargs=[qubit])

@ShellyGarion ShellyGarion mentioned this pull request Jun 18, 2024
Merged
@coveralls
Copy link

coveralls commented Jun 18, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9568639056

Details

  • 201 of 240 (83.75%) changed or added relevant lines in 7 files are covered.
  • 70 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.02%) to 89.777%
Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/circuit/src/operations.rs 185 224 82.59%
Files with Coverage Reduction New Missed Lines %
qiskit/dagcircuit/dagdepnode.py 1 76.81%
crates/qasm2/src/expr.rs 1 94.02%
qiskit/transpiler/passes/optimization/template_matching/template_substitution.py 2 92.67%
crates/qasm2/src/lex.rs 3 93.13%
qiskit/circuit/quantumcircuit.py 14 95.69%
crates/circuit/src/operations.rs 49 68.67%
Totals Coverage Status
Change from base Build 9562946226: 0.02%
Covered Lines: 63578
Relevant Lines: 70818
💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 18, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9569020887

Details

  • 175 of 181 (96.69%) changed or added relevant lines in 6 files are covered.
  • 21 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.002%) to 89.755%
Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/circuit/src/operations.rs 160 166 96.39%
Files with Coverage Reduction New Missed Lines %
qiskit/dagcircuit/dagdepnode.py 1 76.81%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py 2 88.2%
qiskit/transpiler/passes/optimization/template_matching/template_substitution.py 2 92.67%
crates/qasm2/src/lex.rs 4 92.62%
crates/qasm2/src/parse.rs 12 97.15%
Totals Coverage Status
Change from base Build 9562946226: 0.002%
Covered Lines: 63563
Relevant Lines: 70818
💛 - Coveralls

mtreinish
mtreinish approved these changes Jun 18, 2024
View reviewed changes
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for tackling these.

crates/circuit/src/operations.rs
(Self::TGate, smallvec![], q2.clone()),
(Self::HGate, smallvec![], q2),
(Self::CXGate, smallvec![], q0_1.clone()),
(Self::TGate, smallvec![], smallvec![Qubit(0)]),
Copy link
Member

Choose a reason for hiding this comment

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

No variable for this one? :)

@mtreinish mtreinish added this pull request to the merge queue Jun 18, 2024
Merged via the queue into Qiskit:main with commit d4e795b Jun 18, 2024
Procatv pushed a commit to Procatv/qiskit-terra-catherines that referenced this pull request Aug 1, 2024
@ElePT @Procatv
Add rust representation for SGates, TGates, and iSwap gate (Qiskit#12598
0caf055 
)

* Add SGate, SdgGate, iSWAP, TGate, and TdgGate to standard gates in rust.
Add missing gate definitions that depended on these gates (marked as todo).

* Add fast path to circuit methods, fix sneaky bugs, unskip cy and sx tests.

* Unskip ccx test too!

* Fix black
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtreinish mtreinish mtreinish approved these changes

Assignees
No one assigned
Labels
Changelog: None Do not include in changelog performance Rust This PR or issue is related to Rust code in the repository
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ElePT @qiskit-bot @coveralls @mtreinish