-
Notifications
You must be signed in to change notification settings - Fork 56
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
Loop sampling #25
Loop sampling #25
Conversation
Codecov Report
@@ Coverage Diff @@
## master #25 +/- ##
==========================================
+ Coverage 95% 95.21% +0.21%
==========================================
Files 8 8
Lines 560 564 +4
==========================================
+ Hits 532 537 +5
+ Misses 28 27 -1
Continue to review full report at Codecov.
|
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.
Awesome @nquesada. I'm in favour of bringing all the function signatures in-line, and making means
optional everywhere.
|
||
np.random.seed(20) | ||
np.random.seed(137) |
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.
How come you needed to change the seed here?
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 just like 137, it is a more quantum electrodynamical number ;)
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.
🤣
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.
@nquesada You have just given me my new favoite random number seed. 👍
probs1[i] = hafnian(mat).real / factpref | ||
probs1[i] = density_matrix_element( | ||
mu_red, V_red, indices, indices, include_prefactor=True, hbar=hbar | ||
).real |
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.
Nice!
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.
👍
assert np.allclose( | ||
rel_freq, probs, rtol=rel_tol / np.sqrt(n_samples), atol=rel_tol / np.sqrt(n_samples) | ||
) | ||
|
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.
great tests
probs1[i] = hafnian(mat).real / factpref | ||
probs1[i] = density_matrix_element( | ||
mu_red, V_red, indices, indices, include_prefactor=True, hbar=hbar | ||
).real |
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.
👍
|
||
np.random.seed(20) | ||
np.random.seed(137) |
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.
@nquesada You have just given me my new favoite random number seed. 👍
…nto loop_sampling
Description of the Change:
Adds the ability to sample from Gaussian states with finite means, and at the same time simplifies the hafnian sampling functions. Also adds tests to the new functionality
Benefits:
Can sample from Gaussian states with non zero means.
Possible Drawbacks:
The syntax is slightly awkward in that the mean vector is (an optional argument) passed after the covariance matrix and the number of samples. Maybe it might be a good idea to use this syntax also in the quantum functions, i.e. make the vector of means of optional.
Related GitHub Issues: