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

catch all multi target uncontrolled gates #214

Merged
merged 6 commits into from
Dec 11, 2024
Merged
Changes from 1 commit
Commits
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
10 changes: 1 addition & 9 deletions qiskit_ionq/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
QuantumRegister,
ClassicalRegister,
)
from qiskit.circuit.library import standard_gates as q_gates

# Use this to get version instead of __version__ to avoid circular dependency.
from importlib_metadata import version
Expand Down Expand Up @@ -124,13 +123,6 @@
"sxdg": "vi",
}

multi_target_uncontrolled_gates = (
q_gates.SwapGate,
q_gates.RXXGate,
q_gates.RYYGate,
q_gates.RZZGate,
)

# https://ionq.com/docs/getting-started-with-native-gates
ionq_native_basis_gates = [
"gpi", # TODO All single qubit gates can transpile into GPI/GPI2
Expand Down Expand Up @@ -244,7 +236,7 @@ def qiskit_circ_to_ionq_circ(
converted["gate"] = instruction_name

# Make sure uncontrolled multi-targets use all qargs.
if isinstance(instruction, multi_target_uncontrolled_gates):
if instruction.num_qubits > 1 and not hasattr(instruction, "num_ctrl_qubits"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

is num_qubits guaranteed to be set?

I don't quite understand why we are changing this and not

if instruction_name not in GATESET_MAP[gateset]:
raise ionq_exceptions.IonQGateError(instruction_name, gateset)
based on the gates you've seen issues with?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Spoke offline: main issue is that where I linked we check gate name, and here we are checking python classes, which might not match in the case of QASM/etc

WDYT about changing to the values from GATESET_MAP that we know should have multiple controls?

This LGTM assuming the num_qubits attribute must exist, if it might not I would change to getattr(instruction, "num_qubits", 0)

converted["targets"] = [
input_circuit.qubits.index(qargs[0]),
input_circuit.qubits.index(qargs[1]),
Expand Down
Loading