Skip to content

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Jul 9, 2024

Summary

This PR adds the with_controlled_gate_array decorator missing in C3SXGate that enables the to_matrix method. This bug was found while porting C3SXGate to Rust (#12659).

Details and comments

I have also noticed that the path of C3SXGate is incorrect. It's currently in qiskit/circuit/library/standard_gates/x.py while it should be in qiskit/circuit/library/standard_gates/sx.py. I did not address this issue in the current PR because I believe the move to the correct location is a breaking change and should probably be left for 2.0 (although most users won't import gates directly from the file).

@ElePT ElePT added the Changelog: Bugfix Include in the "Fixed" section of the changelog label Jul 9, 2024
@ElePT ElePT requested a review from a team as a code owner July 9, 2024 08:52
@qiskit-bot
Copy link
Collaborator

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

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

@ElePT ElePT force-pushed the fix-c3s-matrix branch from 50abca7 to 4615bc5 Compare July 9, 2024 08:56
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Thanks for the catch. Looking (very briefly) at #10296, it seems like the array was always missing, even before the decorator.

@@ -591,7 +593,7 @@ def __init__(
unit="dt",
_base_label=None,
):
"""Create a new 3-qubit controlled sqrt-X gate.
"""Create a new 3-qubit contƒCrolled sqrt-X gate.
Copy link
Member

Choose a reason for hiding this comment

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

wtf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when I switch keyboards option and command are reversed and stuff happens XD

---
fixes:
- |
Fixed a missing decorator in :class:`.C3SXGate` that made it fail if `gate.to_matrix()` was called.
Copy link
Member

Choose a reason for hiding this comment

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

You can link :meth:`.Gate.to_matrix` to get the proper cross-ref.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 9854198448

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 17 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.007%) to 89.839%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 5 91.6%
crates/qasm2/src/parse.rs 12 96.23%
Totals Coverage Status
Change from base Build 9846433295: -0.007%
Covered Lines: 65645
Relevant Lines: 73070

💛 - Coveralls

@jakelishman jakelishman enabled auto-merge July 9, 2024 09:36
@jakelishman
Copy link
Member

@Mergifyio backport stable/0.46 stable/1.1

Copy link
Contributor

mergify bot commented Jul 9, 2024

backport stable/0.46 stable/1.1

✅ Backports have been created

@jakelishman jakelishman added this pull request to the merge queue Jul 9, 2024
Merged via the queue into Qiskit:main with commit d86f995 Jul 9, 2024
mergify bot pushed a commit that referenced this pull request Jul 9, 2024
* Fix c3sx matrix

* Apply Jake's suggestion

(cherry picked from commit d86f995)

# Conflicts:
#	test/python/circuit/test_rust_equivalence.py
mergify bot pushed a commit that referenced this pull request Jul 9, 2024
* Fix c3sx matrix

* Apply Jake's suggestion

(cherry picked from commit d86f995)

# Conflicts:
#	test/python/circuit/test_rust_equivalence.py
github-merge-queue bot pushed a commit that referenced this pull request Jul 9, 2024
* Fix `C3SXGate` `to_matrix` method (#12742)

* Fix c3sx matrix

* Apply Jake's suggestion

(cherry picked from commit d86f995)

# Conflicts:
#	test/python/circuit/test_rust_equivalence.py

* Remove rust test

---------

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <epenatap@gmail.com>
github-merge-queue bot pushed a commit that referenced this pull request Jul 9, 2024
* Fix `C3SXGate` `to_matrix` method (#12742)

* Fix c3sx matrix

* Apply Jake's suggestion

(cherry picked from commit d86f995)

# Conflicts:
#	test/python/circuit/test_rust_equivalence.py

* Remove rust test

---------

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Elena Peña Tapia <epenatap@gmail.com>
Procatv pushed a commit to Procatv/qiskit-terra-catherines that referenced this pull request Aug 1, 2024
* Fix c3sx matrix

* Apply Jake's suggestion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants