Skip to content

Add check based on maximum number of pairs#104

Merged
Luthaf merged 6 commits intoLuthaf:mainfrom
RMeli:cell-list-bug
Feb 9, 2026
Merged

Add check based on maximum number of pairs#104
Luthaf merged 6 commits intoLuthaf:mainfrom
RMeli:cell-list-bug

Conversation

@RMeli
Copy link
Contributor

@RMeli RMeli commented Feb 4, 2026

The maximum number of pairs that can be handled is limited by VESIN_CUDA_MAX_PAIRS_PER_POINT. The following tests was failing with a large cutoff:

import cupy as cp
import numpy as np

from vesin import NeighborList

dtype = cp.float64

box = cp.eye(3, dtype=dtype) * 3.0
points = cp.array(np.random.default_rng(0).random((100, 3), dtype=dtype) * 3.0)
print(len(points))

calculator = NeighborList(cutoff=4.0, full_list=True, algorithm="cell_list")
i, j, s, D, d = calculator.compute(points, box, True, "ijSDd")

This PR introduces bound checks preventing out of bound accesses.

Copy link
Owner

@Luthaf Luthaf left a comment

Choose a reason for hiding this comment

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

Thanks, this looks very good! I think the example with cutoff=4 and 100 points should work, so we might have to also increase VESIN_CUDA_MAX_PAIRS_PER_POINT anyway.

@RMeli
Copy link
Contributor Author

RMeli commented Feb 5, 2026

Thant can be done easily. What do you think would be a reasonable threshold? In this case the length of the neighbor list was about 100k.

@Luthaf
Copy link
Owner

Luthaf commented Feb 6, 2026

So thinking a bit more about it, the density in this example was kinda insane. A more reasonable example, with the density of osmium, gives me around 200 pairs per atom for a cutoff equal to the cell size. Since the number of pairs will scale with the cutoff cube, I guess something like

max_pairs = 256 * (cutoff / smallest_distance_between_box_faces)**3

Should be fine? It seems to always be larger than the number of pairs for some tests I ran.

@Luthaf
Copy link
Owner

Luthaf commented Feb 6, 2026

And then we can change the example to use a cell of 11A, with a cutoff of 14 A

@RMeli
Copy link
Contributor Author

RMeli commented Feb 6, 2026

So thinking a bit more about it, the density in this example was kinda insane.

Yes, exactly. But was not sure what your requirement are (like doing some crazy high-pressure stuff...).

It seems to always be larger than the number of pairs for some tests I ran.

Currently, max_pairs is computed using the hard-coded VESIN_CUDA_MAX_PAIRS_PER_POINT. Are you suggesting to make the maximum number of pairs flexible at runtime?

@Luthaf
Copy link
Owner

Luthaf commented Feb 6, 2026

Are you suggesting to make the maximum number of pairs flexible at runtime?

I think this makes more sense, no? This way we don't waste VRAM when we have a small cutoff, but we don't crash unless something very wrong is going on

Copy link
Owner

@Luthaf Luthaf left a comment

Choose a reason for hiding this comment

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

Merging this for now, and we can add a dynamic max_pair in a separate PR

@Luthaf Luthaf merged commit 97f5976 into Luthaf:main Feb 9, 2026
11 checks passed
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.

2 participants