-
Notifications
You must be signed in to change notification settings - Fork 51
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
[GQSP] Negative powers and Hamiltonian simulation #669
Conversation
37b9fc7
to
7cccd4c
Compare
3c989ba
to
c7b6337
Compare
4928ba4
to
287efdf
Compare
@tanujkhattar I've implemented the hamiltonian simulation now, along with the call graph. I added unittests with randomly generated select and prepare oracles. I'm still debugging why the expected block doesn't match e^iHt. In the meantime could you review the rest of the code? |
I'll take a look soon, thanks for the update |
4bf5990
to
2264b4b
Compare
@cached_property | ||
def gqsp(self) -> GeneralizedQSP: | ||
return GeneralizedQSP.from_qsp_polynomial( | ||
self.walk_operator, self.approx_cos, negative_power=self.degree |
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've narrowed down the issue to this: as the
Mathematically this makes sense as the only polynomial s.t.
7a8369f
to
fc01bc9
Compare
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.
Left a round of comments for code organization and cleanup while I think more about the hamiltonian simulation issue. Thanks for doing this!
@@ -239,9 +260,11 @@ def safe_angle(x): | |||
class GeneralizedQSP(GateWithRegisters): |
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.
Let's create a new subpackage for qualtran/bloqs/qsp/
and move the generalized qsp code inside the new folder. We can have two files right now
qualtran/bloqs/qsp/generalized_qsp.py
- This should haveGeneralizedQSP
,gqsp_phase_factors
,gqsp_complementary_polynomial
(renameqsp
togqsp
in the last 2 functions)qualtran/bloqs/qsp/polynomial_approximations.py
- This file can have functions which return coefficeints for commonly used polynomial approximations. For example:approx_exp_cos(precision, scaling_const)
andapprox_exp_sin(precision, scaling_const)
, which returns coefficients of the Jacobi-anger polynomial approximation which can be called directly from the HamiltonianSimulationViaGQSP Bloq.
For HamiltonianSimulationByGQSP
, we can create a new subpackage for bloqs/hamiltonian_simulation/
and move HamiltonianSimulationByGQSP
there. But this can be done once the above refactorings are complete to avoid merge conflicts.
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.
For GeneralizedQSP
, add a bloq_example
, autogenerate the notebook from docstrings (you can add manually add more cells and they'll be preserve during the autogeneration process), add bloq_autotester
tests and assert_decompose_is_consistent_with_t_complexity
.
Does it make sense to show some kind of symbolic analysis in GQSP? The current API won't support it since we expect P
and Q
to be tuples of complex numbers and we currently don't have a way to specify sequences of variable symbolic lengths but we can file an issue to track this if there's a nice example where users would benefit from seeing the cost of applying GQSP as in terms of symbolic parameters they care about (for example hamiltonian simulation).
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.
No description provided.