Skip to content

Implement Negate (two's complement) #1144

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

Merged
merged 8 commits into from
Jul 17, 2024

Conversation

anurudhp
Copy link
Contributor

No description provided.

@anurudhp anurudhp requested a review from tanujkhattar July 16, 2024 21:20
Comment on lines +27 to +28
# TODO use QInt once classical sim is fixed.
# classical sim currently only supports unsigned.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this true? I thought classical simulation supports both signed and unsigned integers. Is there an open issue you are referring to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, looks like it's bottlenecked by speed and #1142 should help fix it?

Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

Looking good. Question about classical sim currently only supportin unsigned types - is this true?

@tanujkhattar tanujkhattar self-assigned this Jul 17, 2024
x: Any signed value stored in two's complement form.
"""

dtype: QDType
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we assert that dtype is a signed type ? So QInt or QFxp(signed=True) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually works for unsigned types as well, it's the unary minus operator in c++. Perhaps we can use the c++ naming convention?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, we should at least have a link explaining the behavior for unsigned types in the docstring. I don't have strong opinions on naming convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated docstring to explain this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will leave the Bloq as Negate for now



@frozen
class Negate(Bloq):
Copy link
Collaborator

@tanujkhattar tanujkhattar Jul 17, 2024

Choose a reason for hiding this comment

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

I think it's also worth adding a Not(Bloq) in arithmetic/bitwise.py which simply computes the bitwise NOT via OnEach(self.dtype.num_qubits, XGate()) and is a natural choice for unsigned types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep this is useful! will add in a follow-up PR and cleanup this decomp then.

Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

LGTM. Please consider adding a bitwise Not Bloq for OnEach(XGate(), self.bitsize) to arithmetic/bitwise.py. This can be done in a follow up PR so we can merge this.

@tanujkhattar tanujkhattar enabled auto-merge (squash) July 17, 2024 19:11
@tanujkhattar tanujkhattar merged commit 59f674d into quantumlib:main Jul 17, 2024
7 checks passed
@anurudhp anurudhp deleted the 2024/07/16-negate-2c branch July 17, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants