-
Notifications
You must be signed in to change notification settings - Fork 47
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
Vectorizing GaussianInt
speed up
#1
Comments
Hi @ljmartin , this is a great suggestion (and vectorizing is always a good option to speed up code). Feel free to submit a PR anytime! Thanks for contributing to this project! |
Hi @ljmartin, what is the tentative time-scale for your PR? I am planning on some updates that might affect that part of the code, and wanted to wait for the PR first. So if you are currently unable to submit it, I might just go ahead and introduce the changes you supposed. Upside: You don't need to worry about it. Downside: GitHub will not give you credit - so I just wanted to check in with you and ask what you think about it. |
Hi @hesther, thanks for the ping - yes Ill submit a PR tomorrow! I was thinking of adding a test first to make sure it does what I think it does, but Ill just test it manually before submitting. |
Thanks! Yeah, I can add a test later- actually there aren't any tests yet (the current dummy test only loads the package) for any code yet, this is also on my todo list. |
Hi, thanks for this great package!
I wanted to speed it up for use in large-scale screening, so vectorized the gaussian integration step:
espsim/espsim/electrostatics.py
Line 114 in 463289c
Here's a minimal-ish example, taken from the
scripts
dir:setup:
vectorized integration function (should just be a drop-in for GaussInt):
test equivalence:
output:
These take ~6ms vs 100µs. The
VecGI
should just be a drop in replacement forGaussInt
. Seems to work for Carbo and Tanimoto, so if it's of interest Im happy to submit a PR.cheers
Lewis
The text was updated successfully, but these errors were encountered: