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.

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

Fix multi-controlled gates in name mapping #2326

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

AlbertJP
Copy link

@AlbertJP AlbertJP commented Mar 5, 2025

Summary

MCSXGate, MCYGate, MCZGate, MCRXGate, MCRYGate, MCRZGate, MCU3Gate, MCUGate, and MCSwapGate can not be instantiated due to a mismatch in the number of qubits with the base gate:

CircuitError: 'The number of control qubits must be in `[1, num_qubits - base_gate.num_qubits]`.'

Details and comments

I came across this issue while writing a unit test for Qiskit/qiskit#13952.

Commit doichanj@4587384 (pull #1995) changed the base gates of these gates from non-controlled gates into singly controlled gates, which means there is one qubit too much in the base gate.

An alternative solution would be to reduce num_ctrl_qubits by one when calling ControlledGate.__init__(), but then the multi-controlled gates can only be created for two or more control qubits and one would need to extract the last qubit from the control state to pass it to the base gate.

AlbertJP and others added 2 commits March 5, 2025 09:01
These gates could not be instantiated due to a mismatch in the
number of qubits with the base gate.
@AlbertJP
Copy link
Author

Unit test is failing because it needs #2327 to support the new Qiskit version.

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.

Overall this looks fine to me, but do we need any unit tests here to validate that these classes work, and more importantly don't regress in the future?

@AlbertJP
Copy link
Author

AlbertJP commented Mar 13, 2025

I have just changed some existing tests to use the MCYGate and MCZGate classes instead of creating new gates. I'll add tests for the others as well.

@AlbertJP
Copy link
Author

All gates from name_mapping.py are now in the tests as well.

@AlbertJP AlbertJP requested a review from mtreinish March 13, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants