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

Maked intermediate additions unsigned in ModAdd #1424

Merged
merged 4 commits into from
Sep 28, 2024

Conversation

fpapa250
Copy link
Contributor

Replicate bug with ModAdd(bitsize=8, mod=17).adjoint().on_classical_vals(x=0,y=0). Basically, we do signed integer addition with registers that have unsigned dtypes. Change the intermediate register dtypes to QInt to perform modular reductions then restore to unsigned int dtypes.

@fpapa250 fpapa250 mentioned this pull request Sep 27, 2024
@NoureldinYosri
Copy link
Contributor

this is the only line that needs changing

y = bb.add(AddK(bitsize=self.bitsize + 1, k=-1 * self.mod, signed=True, cvs=()), x=y)

@fpapa250
Copy link
Contributor Author

Both lines ended up needing changing; I tested it with the error showing up in my ECAdd PR.

@NoureldinYosri
Copy link
Contributor

NoureldinYosri commented Sep 28, 2024

Both lines ended up needing changing; I tested it with the error showing up in my ECAdd PR.

yea, makes sense that all additions are unsigned

@NoureldinYosri NoureldinYosri changed the title Fix bug with ModAdd (and adjoint) where negative integer values were being stored in unsigned int registers. Maked intermediate additions unsigned in ModAdd Sep 28, 2024
@NoureldinYosri NoureldinYosri enabled auto-merge (squash) September 28, 2024 07:27
@NoureldinYosri NoureldinYosri enabled auto-merge (squash) September 28, 2024 07:31
@NoureldinYosri NoureldinYosri merged commit b14f1b4 into quantumlib:main Sep 28, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants