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

Make neighbor list compatible with float16 and bfloat16 #273

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

RaulPPelaez
Copy link
Collaborator

This PR adds overloads for float16 and bfloat16 input/output of the neighbor extension.

Makes it possible to train in float16 or bfloat16 without casting in between.

@guillemsimeon
Copy link
Collaborator

do you want me to review?

@RaulPPelaez
Copy link
Collaborator Author

no, I will let you know. Thanks

@RaulPPelaez
Copy link
Collaborator Author

I cannot enable the neighbor tests for float16. Some test always fails because with the precision being so low some there is a disagreement between the reference in the number of neighbors.

I am not sure exactly why. My guess is that, given that the references are being computed with numpy, they are actually being computed in a higher precision, while the other implementations use float16 all the way.

The tests run with random clouds of points, so I can see things being too close to the cutoff distance for this to be a problem and
float16(r)<=float16(rcut) != float64(r)<=float64(rcut)

This begs the question of whether it makes sense to ask for the neighbors in such a low precision....

@RaulPPelaez
Copy link
Collaborator Author

The majority of failing tests yield more neighbor pairs than the reference:

FAILED test_neighbors.py::test_neighbors[dtype0-None-True-True-4.9-128-cuda-brute] - AssertionError: Found num_pairs(118907) > max_num_pairs(118873)
FAILED test_neighbors.py::test_neighbors[dtype0-None-True-True-4.9-128-cuda-shared] - AssertionError: Found num_pairs(118907) > max_num_pairs(118873)
FAILED test_neighbors.py::test_neighbors[dtype0-None-True-False-4.9-128-cuda-brute] - AssertionError: Found num_pairs(112430) > max_num_pairs(112396)
FAILED test_neighbors.py::test_neighbors[dtype0-None-True-False-4.9-128-cuda-shared] - AssertionError: Found num_pairs(112430) > max_num_pairs(112396)
FAILED test_neighbors.py::test_neighbors[dtype0-None-False-True-4.9-128-cuda-brute] - AssertionError: Found num_pairs(62692) > max_num_pairs(62675)
FAILED test_neighbors.py::test_neighbors[dtype0-None-False-True-4.9-128-cuda-shared] - AssertionError: Found num_pairs(62692) > max_num_pairs(62675)
FAILED test_neighbors.py::test_neighbors[dtype0-None-False-False-4.9-128-cuda-brute] - AssertionError: Found num_pairs(56215) > max_num_pairs(56198)
FAILED test_neighbors.py::test_neighbors[dtype0-None-False-False-4.9-128-cuda-shared] - AssertionError: Found num_pairs(56215) > max_num_pairs(56198)
FAILED test_neighbors.py::test_neighbors[dtype0-triclinic-True-True-3.0-128-cuda-brute] - AssertionError: assert (2, 54285) == (2, 54287)
FAILED test_neighbors.py::test_neighbors[dtype0-triclinic-True-True-3.0-128-cuda-shared] - AssertionError: assert (2, 54285) == (2, 54287)
FAILED test_neighbors.py::test_neighbors[dtype0-triclinic-True-True-4.9-1-cuda-brute] - AssertionError: Found num_pairs(4414) > max_num_pairs(4412)
FAILED test_neighbors.py::test_neighbors[dtype0-triclinic-True-True-4.9-1-cuda-shared] - AssertionError: Found num_pairs(4414) > max_num_pairs(4412)
FAILED test_neighbors.py::test_neighbors[dtype0-triclinic-True-True-4.9-2-cuda-brute] - AssertionError: Found num_pairs(4887) > max_num_pairs(4885)
FAILED test_neighbors.py::test_neighbors[dtype0-triclinic-True-True-4.9-2-cuda-shared] - AssertionError: Found num_pairs(4887) > max_num_pairs(4885)
FAILED test_neighbors.py::test_neighbors[dtype0-triclinic-True-True-4.9-3-cuda-brute] - AssertionError: Found num_pairs(5089) > max_num_pairs(5087)

@peastman
Copy link
Collaborator

Building neighbor lists in low precision requires some care. You need to make sure the calculated distance always gets rounded down, never up, so it errs on the side of including extra pairs rather than omitting pairs. Then you need to make sure any code that uses the neighbor list can tolerate extra pairs that are beyond the cutoff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants