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

Hermite improve #93

Merged
merged 18 commits into from
Nov 20, 2019
Merged

Hermite improve #93

merged 18 commits into from
Nov 20, 2019

Conversation

nquesada
Copy link
Collaborator

Improving the speed of the hermite polynomials and also changing their definition
Context:

Description of the Change:

Benefits:

Possible Drawbacks:
The docs need to be updated to reflect the new definition

Related GitHub Issues:

@codecov
Copy link

codecov bot commented Nov 18, 2019

Codecov Report

Merging #93 into master will increase coverage by 0.44%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
+ Coverage   95.75%   96.19%   +0.44%     
==========================================
  Files          11       11              
  Lines         800      894      +94     
==========================================
+ Hits          766      860      +94     
  Misses         34       34
Impacted Files Coverage Δ
thewalrus/quantum.py 98.88% <100%> (+0.98%) ⬆️
thewalrus/_hermite_multidimensional.py 100% <100%> (+2%) ⬆️
thewalrus/samples.py 86.56% <0%> (+1.21%) ⬆️
thewalrus/reference.py 100% <0%> (+1.26%) ⬆️

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 86a58fa...4fdf902. Read the comment docs.

@nquesada nquesada added the WIP label Nov 18, 2019
@nquesada nquesada added review-ready and removed WIP labels Nov 19, 2019
@nquesada nquesada requested a review from josh146 November 19, 2019 16:43
@nquesada
Copy link
Collaborator Author

Fun fact. The very first implementation of tensor_fock required around 180 ms to calculate the Fock representation of an S2gate up to cutoff=6. This version takes about 5 ms

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

💯

@@ -21,6 +21,7 @@ All probability amplitudes of a pure Gaussian state
All matrix elements of a general Gaussian state :func:`thewalrus.quantum.density_matrix()`
A particular probability amplitude of pure Gaussian state :func:`thewalrus.quantum.pure_state_amplitude()`
A particular matrix element of a general Gaussian state :func:`thewalrus.quantum.density_matrix_element()`
The Fock representation of a Gaussian unitary :func:`thewalrus.quantum.fock_tensor()`
================================================================================ =============================
Copy link
Member

Choose a reason for hiding this comment

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

💯

for (int ii = 0; ii < dim; ii++) {
H[nextCoordinate] = H[nextCoordinate] + R(k, ii) * y(ii, 0);
}
H[nextCoordinate] = H[nextCoordinate] + y(k, 0);
Copy link
Member

Choose a reason for hiding this comment

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

Was + R(k, ii) * y(ii, 0) the original matrix multiplication that is now redundant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes!

@@ -776,7 +776,7 @@ TEST(LoopHafnianEigenComplex, EvenRandom) {

}

// Check loop hafnian with eignevalues for complex matrices with odd dimensions.
// Check loop hafnian with eigenvalues for complex matrices with odd dimensions.
Copy link
Member

Choose a reason for hiding this comment

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

lol good catch

@nquesada nquesada merged commit b7a0284 into master Nov 20, 2019
@nquesada nquesada deleted the hermite_improve branch November 20, 2019 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants