-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New function to negate expressions #15447
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
base: main
Are you sure you want to change the base?
Conversation
|
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:
|
| float, unsigned int and duration expressions. Incase a non-expression is passed, it is automatically | ||
| lifted to the correct type. | ||
| Examlpe usage:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @Shobhit21287 , thank you so much for your contribution! there is a small typo in this line, should be Example*
Also, perhaps you should leave a new line at the end of the release note, are the docs still compiling?
| Duration()) | ||
| """ | ||
| operand = lift(operand) | ||
| if operand.type.kind not in (types.Uint, types.Float, types.Duration): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Uint here may be problematic and can lead to undefined or unexpected behavior, because unsigned integers cannot represent negative values (a user expecting a signed integer or an error may end up getting back a large unsigned integer (2**3) - value which is an ambiguous value, and a silent failure). While negation is unambiguous for Float and Duration, negating a Uint requires choosing a specific interpretation like 2's complement, to negate which, a specialized method may need to be defined.
So for now, perhaps Uint could be removed from the check, or specialized logic can be added? Do let me know what you think! Thank you
aaryav-3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Shobhit21287 , this work is a great addition to qiskit, your PR is well written and clean. I had just some clarifications before it looks good to go
Closes #13966
Added a
negatefunction forexpr.Exprletting users negate expressions of typefloat,Uint,duration.The following have been done