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

Adding fock gkps #553

Merged
merged 37 commits into from Mar 12, 2021
Merged

Adding fock gkps #553

merged 37 commits into from Mar 12, 2021

Conversation

nquesada
Copy link
Collaborator

@nquesada nquesada commented Mar 8, 2021

Adds preparation methods for the GKP states in the Fock backend

@nquesada nquesada marked this pull request as draft March 8, 2021 22:06
@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #553 (80170e6) into master (9255982) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #553   +/-   ##
=======================================
  Coverage   98.23%   98.24%           
=======================================
  Files          76       76           
  Lines        8397     8429   +32     
=======================================
+ Hits         8249     8281   +32     
  Misses        148      148           
Impacted Files Coverage Δ
strawberryfields/compilers/fock.py 100.00% <ø> (ø)
...trawberryfields/backends/bosonicbackend/backend.py 100.00% <100.00%> (ø)
strawberryfields/backends/fockbackend/backend.py 100.00% <100.00%> (ø)
strawberryfields/backends/fockbackend/circuit.py 98.42% <100.00%> (+0.02%) ⬆️
strawberryfields/backends/fockbackend/ops.py 94.68% <100.00%> (+0.63%) ⬆️
strawberryfields/ops.py 98.89% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9255982...80170e6. Read the comment docs.

@@ -383,7 +383,7 @@ def prepare_gaussian_state(self, r, V, modes):
self.circuit.from_covmat(cov, modes)
self.circuit.from_mean(means, modes)

def prepare_cat(self, alpha, phi, representation, cutoff, D):
def prepare_cat(self, alpha, phi, representation, amplcutoff, D):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that in the original PR for the bosonic backend we use cutoff for the small real number used to decide whether to include a Gaussian in phase space. This is inconsistent with the use of cutoff for the Fock space truncation thus I've renamed it in the bosonic backend.

@nquesada nquesada marked this pull request as ready for review March 10, 2021 00:47
Copy link
Collaborator

@elib20 elib20 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! I have mostly minor formatting comments, and one more substantial question about normalization. Once the latter is resolved it's good to go from my end.

strawberryfields/backends/bosonicbackend/backend.py Outdated Show resolved Hide resolved
strawberryfields/backends/bosonicbackend/backend.py Outdated Show resolved Hide resolved
strawberryfields/backends/fockbackend/backend.py Outdated Show resolved Hide resolved
strawberryfields/backends/fockbackend/ops.py Outdated Show resolved Hide resolved
tests/frontend/test_ops_gate.py Outdated Show resolved Hide resolved
strawberryfields/backends/fockbackend/ops.py Outdated Show resolved Hide resolved
@nquesada nquesada requested review from elib20 and thisac and removed request for thisac and antalszava March 11, 2021 18:40
Co-authored-by: elib20 <53090166+elib20@users.noreply.github.com>
Co-authored-by: elib20 <53090166+elib20@users.noreply.github.com>
Copy link
Contributor

@antalszava antalszava left a comment

Choose a reason for hiding this comment

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

Looks good to me! 💯 Some comments, none major. Just curious: none of the functions would be user-facing, correct? Just for how they appear in the docs.

strawberryfields/backends/fockbackend/backend.py Outdated Show resolved Hide resolved
strawberryfields/backends/fockbackend/ops.py Outdated Show resolved Hide resolved


@functools.lru_cache()
def squaregkpState(theta, phi, epsilon, ampl_cutoff, cutoff):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def squaregkpState(theta, phi, epsilon, ampl_cutoff, cutoff):
def square_gkp_state(theta, phi, epsilon, ampl_cutoff, cutoff):

strawberryfields/backends/fockbackend/ops.py Show resolved Hide resolved
strawberryfields/backends/fockbackend/ops.py Outdated Show resolved Hide resolved
tests/integration/test_utils_integration.py Show resolved Hide resolved
tests/integration/test_utils_integration.py Show resolved Hide resolved
tests/integration/test_utils_integration.py Show resolved Hide resolved
strawberryfields/backends/fockbackend/ops.py Show resolved Hide resolved
strawberryfields/backends/fockbackend/ops.py Outdated Show resolved Hide resolved
nquesada and others added 3 commits March 11, 2021 19:07
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Copy link
Collaborator

@elib20 elib20 left a comment

Choose a reason for hiding this comment

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

Very minor suggestions. Looks good to me!

strawberryfields/backends/fockbackend/backend.py Outdated Show resolved Hide resolved
strawberryfields/backends/fockbackend/ops.py Outdated Show resolved Hide resolved
strawberryfields/ops.py Show resolved Hide resolved
@nquesada nquesada merged commit d8528d4 into master Mar 12, 2021
@nquesada nquesada deleted the adding_fock_gkps branch March 12, 2021 15:52
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.

None yet

3 participants