Speed up coordgen, especially for very slow molecules #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ricrogz
approved these changes
Sep 23, 2019
|
oh, bummer. It looks like the I'll back out that part of the change. |
This improves build stability by reducing the scope of rebuilds required when a header changes.
d-b-w
force-pushed
the
dbn_performance
branch
2 times, most recently
from
30cef9b
to
0004435
Compare
|
I pulled out the refactorings I was doing to try to understand things into a separate set of commits. This is now only the performance improvement, a test, and a change to remove some unused headers to speed up my rebuilds. |
|
@d-b-w, the CI builds failed, apparently, because of an unused variable (travis) and some |
|
Thanks, @ricrogz . I see 'em. |
When detecting clashes, do a rough binning of atoms. If both atoms for one bond are above both atoms of the other bond, they can't be a clash. This saves clash calculations for things that are obviously not clashes. It has an impact on big structures that are partially templated. (overall 15% speedup, but 40% improvement for the worst molecules in the test case. Also adds a simplistic test that the coordinates generated don't have crazy bond lengths.
d-b-w
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improve coordgen performance: #39
When detecting clashes, do a rough binning of atoms. If both
atoms for one bond are above both atoms of the other bond, they
can't be a clash. This saves clash calculations for things that
are obviously not clashes. It has an impact on big structures
that are partially templated. (overall 15% speedup, but 40%
improvement for the worst molecules in the test case.
coordgen is still slower than RDkit's native coordinate
generation, though.
Also adds a simple test that the coordinates generated don't
have crazy bond lengths. This would have caught my earlier
misunderstanding.