Skip to content

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Mar 27, 2025

Summary

Normally creating a custom gate class that overloads the name of a Qiskit defined operation is not valid and not allowed. The names have meaning and are often used as identifiers and this overloading the name will prevent Qiskit from correctly identifying an operation. However as was discovered in #14103 there are some paths available around serialization and I/O where Qiskit does this itself. For example, qasm (both 2 and 3) is a lossy serialization format and qasm2 doesn't have a representation of a UnitaryGate. So when the qasm2 exporter encounteres a UnitaryGate it is serialized as a custom gate definition with the name "unitary" in the output qasm2 and the definition is a decomposition of the unitary from the UnitaryGate. When that qasm2 program is subsequently deserialized by qiskit parser the custom gate named "unitary" is added as a _DefinedGate subclass which includes an __array__ implementation which computes the unitary from the definition using the quantum info Operator class. This makes the custom gate parsed from qasm2 look like a UnitaryGate despite not actually one so this is typically fine for most use cases. However, since #13759 trying to add that not UnitaryGate object named "unitary" would cause the Python -> Rust translation to panic (which happens as part of qasm2 desierailzation). because the conversion was expecting a gate named unitary to be a UnitaryGate as is prescribed by the data model.

This commit fixes this by gracefully handling the lack of a matrix parameter as it not actually being a UnitaryGate and instead the object gets treated as a PyGate in rust which is the expected behavior.

Details and comments

Related to #14103


This is an automatic backport of pull request #14109 done by Mergify.

Normally creating a custom gate class that overloads the name of a
Qiskit defined operation is not valid and not allowed. The names have
meaning and are often used as identifiers and this overloading the name
will prevent Qiskit from correctly identifying an operation. However as
was discovered in #14103 there are some paths available around
serialization and I/O where Qiskit does this itself. For example, qasm
(both 2 and 3) is a lossy serialization format and qasm2 doesn't have a
representation of a UnitaryGate. So when the qasm2 exporter encounteres
a `UnitaryGate` it is serialized as a custom gate definition with the
name "unitary" in the output qasm2 and the definition is a decomposition
of the unitary from the `UnitaryGate`. When that qasm2 program is
subsequently deserialized by qiskit parser the custom gate named "unitary"
is added as a `_DefinedGate` subclass which includes an `__array__`
implementation which computes the unitary from the definition using
the quantum info Operator class. This makes the custom gate parsed from
qasm2 look like a `UnitaryGate` despite not actually one so this is
typically fine for most use cases. However, since #13759 trying to add
that not `UnitaryGate` object named "unitary" would cause the Python ->
Rust translation to panic (which happens as part of qasm2
desierailzation). because the conversion was expecting a gate named
`unitary` to be a `UnitaryGate` as is prescribed by the data model.

This commit fixes this by gracefully handling the lack of a matrix
parameter as it not actually being a `UnitaryGate` and instead the
object gets treated as a `PyGate` in rust which is the expected
behavior.

Related to #14103

(cherry picked from commit ddb8025)

# Conflicts:
#	crates/circuit/src/circuit_instruction.rs
#	test/python/transpiler/test_split_2q_unitaries.py
@mergify mergify bot requested a review from a team as a code owner March 27, 2025 14:33
@mergify mergify bot added the conflicts used by mergify when there are conflicts in a port label Mar 27, 2025
Copy link
Contributor Author

mergify bot commented Mar 27, 2025

Cherry-pick of ddb8025 has failed:

On branch mergify/bp/stable/1.4/pr-14109
Your branch is up to date with 'origin/stable/1.4'.

You are currently cherry-picking commit ddb802506.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   test/python/qasm2/test_structure.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   crates/circuit/src/circuit_instruction.rs
	both modified:   test/python/transpiler/test_split_2q_unitaries.py

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

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

  • @Qiskit/terra-core

@github-actions github-actions bot added mod: qasm2 Relating to OpenQASM 2 import or export Changelog: None Do not include in changelog Rust This PR or issue is related to Rust code in the repository labels Mar 27, 2025
@github-actions github-actions bot added this to the 2.0.0 milestone Mar 27, 2025
@raynelfss
Copy link
Contributor

This PR is not to be backported to 1.4 because UnitaryGate in rust is not present. Refer to #14108 for a proper fix

@raynelfss raynelfss closed this Mar 27, 2025
@jakelishman jakelishman deleted the mergify/bp/stable/1.4/pr-14109 branch June 9, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog conflicts used by mergify when there are conflicts in a port mod: qasm2 Relating to OpenQASM 2 import or export Rust This PR or issue is related to Rust code in the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants