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

Expose C++ arrays to Python without data copying #123

Merged
merged 26 commits into from
Jan 17, 2020
Merged

Conversation

nquesada
Copy link
Collaborator

Now the returns of the hermite functions are not copied but are passed transparently into numpy arrays

@codecov
Copy link

codecov bot commented Jan 14, 2020

Codecov Report

Merging #123 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #123   +/-   ##
=======================================
  Coverage   97.72%   97.72%           
=======================================
  Files          12       12           
  Lines         881      881           
=======================================
  Hits          861      861           
  Misses         20       20

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 e13a0d6...6c74ccb. Read the comment docs.

@nquesada
Copy link
Collaborator Author

This is a benchmarking of the operations in thewalrus.fock_tensor vs the operations in fock.ops in strawberryfields:
image

@nquesada
Copy link
Collaborator Author

Wonder if it is worth precomputing the first 50 or so square roots of integers and not have to calculate them on the go.

@nquesada nquesada removed the request for review from josh146 January 16, 2020 18:04
@josh146 josh146 added the C++ label Jan 16, 2020
@josh146 josh146 changed the title Better c calls Expose C++ arrays to Python without data copying Jan 16, 2020
thewalrus/libwalrus.pyx Outdated Show resolved Hide resolved
@josh146 josh146 added the python label Jan 16, 2020
Copy link
Contributor

@trevor-vincent trevor-vincent left a comment

Choose a reason for hiding this comment

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

Thanks Nico, I flagged a couple of possible optimizations. Let me know what you think.

include/hermite_multidimensional.hpp Outdated Show resolved Hide resolved
include/hermite_multidimensional.hpp Outdated Show resolved Hide resolved
include/hermite_multidimensional.hpp Show resolved Hide resolved
include/hermite_multidimensional.hpp Show resolved Hide resolved
include/hermite_multidimensional.hpp Show resolved Hide resolved
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.

Really nice :)

include/hermite_multidimensional.hpp Show resolved Hide resolved
include/hermite_multidimensional.hpp Outdated Show resolved Hide resolved
include/hermite_multidimensional.hpp Outdated Show resolved Hide resolved
include/hermite_multidimensional.hpp Outdated Show resolved Hide resolved
include/hermite_multidimensional.hpp Show resolved Hide resolved
include/hermite_multidimensional.hpp Outdated Show resolved Hide resolved
include/hermite_multidimensional.hpp Outdated Show resolved Hide resolved
thewalrus/libwalrus.pyx Outdated Show resolved Hide resolved
thewalrus/libwalrus.pyx Show resolved Hide resolved
thewalrus/libwalrus.pyx Show resolved Hide resolved
@nquesada nquesada merged commit 9fcb7a3 into master Jan 17, 2020
@nquesada nquesada deleted the better_c_calls branch January 21, 2020 14:45
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.

3 participants