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

Optimize gate count of Subtract to n-1 Toffolis #1057

Merged
merged 25 commits into from
Jul 19, 2024
Merged

Optimize gate count of Subtract to n-1 Toffolis #1057

merged 25 commits into from
Jul 19, 2024

Conversation

NoureldinYosri
Copy link
Contributor

@NoureldinYosri NoureldinYosri commented Jun 7, 2024

This uses the bit-magic a-b = ~(~a + b)

@NoureldinYosri NoureldinYosri marked this pull request as draft June 7, 2024 20:06
@NoureldinYosri NoureldinYosri marked this pull request as ready for review June 7, 2024 21:31
@NoureldinYosri NoureldinYosri requested a review from mpharrigan June 7, 2024 22:59
@mpharrigan
Copy link
Collaborator

This uses the bit-magic a-b = ~(~a + b)

would be useful to include in the description

Copy link
Collaborator

@mpharrigan mpharrigan left a comment

Choose a reason for hiding this comment

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

docstring!

qualtran/bloqs/arithmetic/subtraction.py Outdated Show resolved Hide resolved
qualtran/bloqs/arithmetic/subtraction_test.py Outdated Show resolved Hide resolved
@mpharrigan
Copy link
Collaborator

@NoureldinYosri can you make the requested changes

@NoureldinYosri
Copy link
Contributor Author

@mpharrigan done, I was getting test failures when doing the classical simulation test because classical simulation tools couldn't handle negative values and both Add and Subtract bloqs had faulty classical action definitions for the case of signed inputs... I fixed all of these issues

@NoureldinYosri
Copy link
Contributor Author

@mpharrigan ptal

Comment on lines 161 to 162
hN = N >> 1
return {'a': a, 'b': int(a + b + hN) % N - hN}
Copy link
Collaborator

Choose a reason for hiding this comment

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

h is half? can you provide a line comment explaining the logic here and consider using a more meaningful name than hN. Was the previous behavior using math.fmod wrong? can you add a test that would have failed with the old logic?


This construction uses `XGate` and `AddK` to compute the twos-compliment of `b` before
doing a standard `Add`.
This construction uses the relation `a - b = ~(~a + b)` to turn the operation into addition.
Copy link
Collaborator

Choose a reason for hiding this comment

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

are there any quantum references where this construction is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tanujkhattar you mentioned that this trick is used in some phase gradient papers, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mpharrigan
Copy link
Collaborator

What happens if a_bitsize is equal to b_bitsize?

This is a good example of a bloq that should support symbolic decomposition; at least for sizeof(a) == sizeof(b)

@NoureldinYosri NoureldinYosri requested a review from mpharrigan July 8, 2024 23:06
@anurudhp
Copy link
Contributor

@NoureldinYosri do take a look at #1161 and #1162, they would help simplify this decomposition

Copy link
Collaborator

@mpharrigan mpharrigan left a comment

Choose a reason for hiding this comment

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

can you address @anurudhp 's comment and maybe open an issue to track

@NoureldinYosri
Copy link
Contributor Author

can you address @anurudhp 's comment and maybe open an issue to track

will do when the other PR is merged

@NoureldinYosri NoureldinYosri enabled auto-merge (squash) July 19, 2024 21:18
@NoureldinYosri NoureldinYosri merged commit 1a0e3ab into quantumlib:main Jul 19, 2024
7 checks passed
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.

4 participants