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

Add missing variable width operations to standard_gate_name_mapping() #8935

Conversation

mtreinish
Copy link
Member

Summary

In #8852 we added support to the target class for representing variable width operations by using the class directly instead of an instance. Now that we have settled on a representation for these types of operations we can add them to the built in name mapping function (from which they were previously excluded. This commit adds the built-in variable width operations to the output of the function and returns just the class instead of an instance.

Details and comments

In Qiskit#8852 we added support to the target class for representing variable
width operations by using the class directly instead of an instance. Now
that we have settled on a representation for these types of operations
we can add them to the built in name mapping function (from which they
were previously excluded. This commit adds the built-in variable width
operations to the output of the function and returns just the class
instead of an instance.
@mtreinish mtreinish requested a review from a team as a code owner October 17, 2022 20:52
@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 the following people are requested to review this:

@mtreinish mtreinish added the Changelog: None Do not include in changelog label Oct 17, 2022
@mtreinish
Copy link
Member Author

I was originally thinking we should backport this, but since it's arguably an API change for the function (since it'll return a class for the variable width ops instead of an instance) so I don't think we should.

from qiskit.circuit.parameter import Parameter
from qiskit.circuit.measure import Measure
from qiskit.circuit.delay import Delay
from qiskit.circuit.reset import Reset
from qiskit.circuit.controlflow import IfElseOp, WhileLoop, ForLoop
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really certain we should be returning these as "standard gates"; they're no gates, and they don't really follow a standard form of accepting n float-like parameters.

I suppose it depends a bit on how the result of this function is meant to be used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was on the fence about this too, but we already had Delay, Reset, etc here. This was added in #8759 so we had a mapping for basis_gate entries to object for a target. I guess the function name isn't great and we should rename it (well, alias, then deprecate, then rename).

qiskit/circuit/library/standard_gates/__init__.py Outdated Show resolved Hide resolved
@mtreinish mtreinish closed this Sep 12, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants