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 RSA Phase Estimate Bloq and Move ModExp to rsa/ subdirectory #1428

Merged
merged 19 commits into from
Oct 18, 2024

Conversation

fpapa250
Copy link
Contributor

I made a higher level top bloq with the phase estimation for shors for RSA. I also moved the bloqs to an rsa/ directory and moved the MeasureQFT shim to the factoring/ directory. Last, I made the ModExp bloq take an (n_exponent,) shaped QBit() register because the controls are used in the plus state in the phase estimation circuit so it can't be of type QUint (I think).

@mpharrigan mpharrigan self-requested a review October 7, 2024 23:44
@mpharrigan
Copy link
Collaborator

I think we discussed offline, but +1 to the top-level bloq. This should be set up as "phase estimation of the modular multiply operation". -1 to the signature change of ModExp, but maybe we don't include modexp as part of the decomposition if we frame the overall algorithm as phase estimation of modmul

@fpapa250 fpapa250 changed the title RSA Enhancements Add RSA Phase Estimate Bloq and Move ModExp to rsa/ subdirectory Oct 12, 2024
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.

some nits, but looking good!

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.

random issues

@mpharrigan mpharrigan enabled auto-merge (squash) October 18, 2024 19:59
@mpharrigan mpharrigan merged commit c227032 into quantumlib:main Oct 18, 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