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

Neighborlists? #2

Open
proteneer opened this issue Sep 18, 2020 · 3 comments
Open

Neighborlists? #2

proteneer opened this issue Sep 18, 2020 · 3 comments
Labels
question Further information is requested

Comments

@proteneer
Copy link

This is a thread for discussing how we should (eventually) deal with neighborlists in ANI.

-should the CPU code have a neighborlist?
-which approach should we take? voxel/spatial hashing? or something more similar to what's done in OpenMM
-should we build a single neighborlist that's the max(radial_cutoff, angular_cutoff), or do something more complicated?

@peastman
Copy link
Member

The current CPU code uses an O(N^2) algorithm to build a neighbor list that get used for computing angles. I plan to do the same for CUDA.

It would be straightforward to replace the O(N^2) search with a voxel algorithm or something similar. Whether that's a good idea depends on what we're optimizing for. Our initial requirements specified that it should be optimized for systems of around 100 atoms. At that size, the full search is quite fast and doing anything more complicated would likely end up being slower. As systems get larger, it will become more of a bottleneck. I expect that by around 1000 atoms it will be significant, but until I get the CUDA implementation done and profile it I won't know. Perhaps even at that size most of the time will still be spent in the angles. We'll see.

@raimis
Copy link
Contributor

raimis commented Sep 21, 2020

Maybe we should consider more modularity. The neighbour list should be a separate operation from the featurizer by itself. So in the future, it is straightforward to replace it with a more efficient implementation if needed and reuse for other NNPs. For example, the SchNet implementation would benefit greatly from an efficient neighbour list operation.

@peastman
Copy link
Member

My goal in the ANI implementation is performance, not modularity. We can have a separate stand-alone neighbor list implementation if that would be useful. ANI has a few properties that make it best to handle separately. 1) It has separate cutoffs for radial and angular features. 2) Most of the time is spent computing the angular features, so it's important to have a neighbor list with the shorter cutoff just for them. 3) Combining the neighbor list building with the radial features lets us build the neighbor list at minimal extra cost.

@raimis raimis added the question Further information is requested label May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants