From d65fabd2aef383690a9bbc4643a761bf059e6b0e Mon Sep 17 00:00:00 2001 From: fncnt Date: Mon, 23 Feb 2026 16:16:39 +0100 Subject: [PATCH 1/2] Remove Send + Sync trait bounds in sequential code (and make them more concise in parallel code) --- src/cellgrid.rs | 7 ++----- src/cellgrid/iters.rs | 11 ++++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/cellgrid.rs b/src/cellgrid.rs index 5c6a46f..166602b 100644 --- a/src/cellgrid.rs +++ b/src/cellgrid.rs @@ -140,8 +140,6 @@ where + ConstZero + AsPrimitive + SimdPartialOrd - + Send - + Sync + std::fmt::Debug + Default, { @@ -322,8 +320,7 @@ where impl, const N: usize, T> CellGrid where - T: Float + ConstOne + AsPrimitive + std::fmt::Debug + NumAssignOps + Send + Sync, - P: Send + Sync, + T: Float + ConstOne + AsPrimitive + std::fmt::Debug + NumAssignOps, { /// Returns an iterator over all relevant (i.e. within cutoff threshold + some extra) unique /// pairs of particles in this `CellGrid`. @@ -413,7 +410,7 @@ where #[cfg(feature = "rayon")] impl CellGrid where - T: Float + NumAssignOps + ConstOne + AsPrimitive + Send + Sync + std::fmt::Debug, + T: Float + NumAssignOps + ConstOne + AsPrimitive + Sync + std::fmt::Debug, P: ParticleLike<[T; N]> + Send + Sync, { /// Returns a parallel iterator over all relevant (i.e. within cutoff threshold + some extra) diff --git a/src/cellgrid/iters.rs b/src/cellgrid/iters.rs index f11fc23..b1d1746 100644 --- a/src/cellgrid/iters.rs +++ b/src/cellgrid/iters.rs @@ -105,8 +105,8 @@ where impl<'g, P, const N: usize, F> GridCell<'g, P, N, F> where - F: Float + NumAssignOps + ConstOne + AsPrimitive + Send + Sync + std::fmt::Debug, - P: ParticleLike<[F; N]> + Send + Sync, + F: Float + NumAssignOps + ConstOne + AsPrimitive + std::fmt::Debug, + P: ParticleLike<[F; N]>, { /// Returns the (flat) cell index of this (possibly empty) `GridCell`. pub(crate) fn index(&self) -> i32 { @@ -208,14 +208,14 @@ where /// This method consumes `self` but `GridCell` implements [`Copy`]. //TODO: handle full-space as well //TODO: document that we're relying on GridCell impl'ing Copy here (so we can safely consume `self`) - pub fn particle_pairs(self) -> impl FusedIterator + Clone + Send + Sync { + pub fn particle_pairs(self) -> impl FusedIterator + Clone { self.intra_cell_pairs().chain(self.inter_cell_pairs()) } } impl CellGrid where - F: Float + NumAssignOps + ConstOne + AsPrimitive + Send + Sync + std::fmt::Debug, + F: Float + NumAssignOps + ConstOne + AsPrimitive + std::fmt::Debug, P: ParticleLike<[F; N]>, { /// Returns an iterator over all [`GridCell`]s in this `CellGrid`, excluding empty cells. @@ -253,7 +253,8 @@ where #[cfg(feature = "rayon")] pub fn par_iter(&self) -> impl ParallelIterator> where - P: Send + Sync, + P: Sync, + F: Sync, { self.cells .par_keys() From 856d0acb5c8863dc2e977664b3afdb46daf9db33 Mon Sep 17 00:00:00 2001 From: fncnt Date: Mon, 13 Apr 2026 11:11:27 +0200 Subject: [PATCH 2/2] API: Make ParticleLike a subtrait of Clone instead of Copy. --- benches/cellgrid.rs | 4 ++-- benches/iters.rs | 2 +- benches/lj.rs | 4 ++-- examples/minimal.rs | 4 ++-- python/src/lib.rs | 6 +++--- src/cellgrid.rs | 33 +++++++++++++++++++++++---------- src/cellgrid/iters.rs | 24 ++++++++++++++---------- src/cellgrid/storage.rs | 2 +- src/lib.rs | 14 +++++++++++--- 9 files changed, 59 insertions(+), 34 deletions(-) diff --git a/benches/cellgrid.rs b/benches/cellgrid.rs index 7af082c..b295e3c 100644 --- a/benches/cellgrid.rs +++ b/benches/cellgrid.rs @@ -82,7 +82,7 @@ pub fn bench_cellgrid(c: &mut Criterion) { let cutoff_squared = cutoff.powi(2); b.iter(|| { cg.particle_pairs() - .filter(|&((_i, p), (_j, q))| { + .filter(|&(&(_i, p), &(_j, q))| { distance_squared(&(*p).into(), &(*q).into()) <= cutoff_squared }) .for_each(|_| {}); @@ -97,7 +97,7 @@ pub fn bench_cellgrid(c: &mut Criterion) { |b, cg| { let cutoff_squared = cutoff.powi(2); b.iter(|| { - cg.par_particle_pairs().for_each(|((_i, p), (_j, q))| { + cg.par_particle_pairs().for_each(|(&(_i, p), &(_j, q))| { if distance_squared(&(*p).into(), &(*q).into()) <= cutoff_squared { } else { } diff --git a/benches/iters.rs b/benches/iters.rs index a76cfb1..d983862 100644 --- a/benches/iters.rs +++ b/benches/iters.rs @@ -91,7 +91,7 @@ pub fn bench_iters(c: &mut Criterion) { let cutoff_squared = cutoff.powi(2); b.iter(|| { pool.install(|| { - cg.par_particle_pairs().for_each(|((_i, p), (_j, q))| { + cg.par_particle_pairs().for_each(|(&(_i, p), &(_j, q))| { if distance_squared(&(*p).into(), &(*q).into()) <= cutoff_squared { } else { } diff --git a/benches/lj.rs b/benches/lj.rs index 2f88e6d..7701531 100644 --- a/benches/lj.rs +++ b/benches/lj.rs @@ -80,7 +80,7 @@ pub fn bench_lj(c: &mut Criterion) { ); let potential_energy: F32or64 = cg .particle_pairs() - .filter_map(|((_i, p), (_j, q))| { + .filter_map(|(&(_i, p), &(_j, q))| { let dsq = distance_squared(&(*p).into(), &(*q).into()); if dsq < cutoff_squared { Some(dsq) @@ -109,7 +109,7 @@ pub fn bench_lj(c: &mut Criterion) { ); let _potential_energy: F32or64 = cg .particle_pairs() - .filter_map(|((_i, p), (_j, q))| { + .filter_map(|(&(_i, p), &(_j, q))| { let dsq = distance_squared(&(*p).into(), &(*q).into()); if dsq < cutoff_squared { Some(dsq) diff --git a/examples/minimal.rs b/examples/minimal.rs index a3bf3c9..bcb2df4 100644 --- a/examples/minimal.rs +++ b/examples/minimal.rs @@ -53,7 +53,7 @@ fn main() { #[cfg(not(feature = "rayon"))] // let count = cg.point_pairs().count(); cg.particle_pairs() - .filter(|&((_i, p), (_j, q))| { + .filter(|&(&(_i, p), &(_j, q))| { distance_squared(&(*p).into(), &(*q).into()) <= _cutoff_squared }) .for_each(|_| black_box(())); @@ -61,7 +61,7 @@ fn main() { #[cfg(feature = "rayon")] cg.par_particle_pairs() - .filter(|&((_i, p), (_j, q))| { + .filter(|&(&(_i, p), &(_j, q))| { distance_squared(&(*p).into(), &(*q).into()) <= _cutoff_squared }) .for_each(|_| { diff --git a/python/src/lib.rs b/python/src/lib.rs index 059a8b3..8c2183c 100644 --- a/python/src/lib.rs +++ b/python/src/lib.rs @@ -236,7 +236,7 @@ impl PyCellGrid { std::array::from_fn(|i| coordinates[i] - other[i]).map(|diff| diff * diff); x + y + z <= cutoff_squared }); - Some(out.collect()) + Some(out.copied().collect()) }) } @@ -286,7 +286,7 @@ impl PyCellGridIter { // let _owner = (&py_cellgrid) // .into_py_any(py) // .expect("could not store owner internally"); - let iter = Box::new((&py_cellgrid).inner.particle_pairs()); + let iter = Box::new((&py_cellgrid).inner.particle_pairs().map(|(&p, &q)| (p, q))); // SAFETY: the idea is that `_keep_borrow` makes sure that `iter`s lifetime can be extended // SAFETY: replicating some ideas from // SAFETY: https://github.com/PyO3/pyo3/issues/1085 and @@ -366,7 +366,7 @@ impl PyCellQueryIter { coordinates: Borrowed<'_, '_, PyAny>, ) -> Option { let coordinates = <[f64; 3] as FromPyObject>::extract(coordinates).ok()?; - let iter = Box::new((&py_cellgrid).inner.query_neighbors(coordinates)?); + let iter = Box::new((&py_cellgrid).inner.query_neighbors(coordinates)?.copied()); // SAFETY: see PyCellGridIter let iter = unsafe { std::mem::transmute::< diff --git a/src/cellgrid.rs b/src/cellgrid.rs index 166602b..bb3f1df 100644 --- a/src/cellgrid.rs +++ b/src/cellgrid.rs @@ -228,7 +228,7 @@ where // FIXME: would just have to make sure that cell is always unique when operating on chunks // FIXME: (pretty much the same issue as with counting cell sizes concurrently) cell_lists.push( - particle, + particle.clone(), cells .get_mut(cell) .expect("cell grid should contain every cell in the grid index"), @@ -308,7 +308,7 @@ where //TODO: see `::rebuild()` .for_each(|(cell, particle)| { self.cell_lists.push( - particle, + particle.clone(), self.cells .get_mut(cell) .expect("cell grid should contain every cell in the grid index"), @@ -333,7 +333,7 @@ where /// let cell_grid = CellGrid::new(data.iter().copied().enumerate(), 1.0); /// cell_grid.particle_pairs() /// // usually, .filter_map() is preferable (so distance computations can be re-used) - /// .filter(|&((_i, p), (_j, q))| { + /// .filter(|&(&(_i, p), &(_j, q))| { /// distance_squared(&p.into(), &q.into()) <= 1.0 /// }) /// .for_each(|((_i, p), (_j, q))| { @@ -341,7 +341,7 @@ where /// }); /// ``` #[must_use = "iterators are lazy and do nothing unless consumed"] - pub fn particle_pairs(&self) -> impl Iterator + Clone { + pub fn particle_pairs(&self) -> impl Iterator + Clone { self.iter().flat_map(|cell| cell.particle_pairs()) } @@ -387,7 +387,7 @@ where /// .expect("the queried particle should be within `cutoff` of this grid's shape") /// // usually, .filter_map() is preferable (so distance computations can be re-used) /// .filter(|&(_j, q)| { - /// distance_squared(&p.into(), &q.into()) <= 1.0 + /// distance_squared(&p.into(), &(*q).into()) <= 1.0 /// }) /// .for_each(|(_j, q)| { /// /* do some work */ @@ -397,14 +397,27 @@ where pub fn query_neighbors>( &self, particle: Q, - ) -> Option + Clone> { + ) -> Option + Clone> { self.query(particle).map(|this| { - this.iter().copied().chain( + this.iter().chain( this.neighbors::() - .flat_map(|cell| cell.iter().copied()), + .flat_map(|cell| cell.iter()), ) }) } + + /// Returns a slice of the internal cell storage. + /// + ///
+ /// + /// This is an experimental item. + /// It might get removed in the future or its usage might change. + /// + ///
+ #[doc(hidden)] + pub fn cell_storage(&self) -> &[P] { + &self.cell_lists.buffer + } } #[cfg(feature = "rayon")] @@ -431,13 +444,13 @@ where /// cell_grid.par_particle_pairs() // TODO: fact-check the statement below: /// // Try to avoid filtering this ParallelIterator to avoid significant overhead: - /// .for_each(|((_i, p), (_j, q))| { + /// .for_each(|(&(_i, p), &(_j, q))| { /// if distance_squared(&p.into(), &q.into()) <= 1.0 { /// /* do some work */ /// } /// }); /// ``` - pub fn par_particle_pairs(&self) -> impl ParallelIterator { + pub fn par_particle_pairs(&self) -> impl ParallelIterator { // TODO: ideally, we would schedule 2 threads for cell.particle_pairs() with the same CPU affinity // TODO: so they can share their resources self.par_iter().flat_map_iter(|cell| cell.particle_pairs()) diff --git a/src/cellgrid/iters.rs b/src/cellgrid/iters.rs index b1d1746..b89ae50 100644 --- a/src/cellgrid/iters.rs +++ b/src/cellgrid/iters.rs @@ -91,7 +91,7 @@ pub mod neighborhood { } /// `GridCell` represents a possibly empty (by construction) cell of a [`CellGrid`]. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub struct GridCell<'g, P, const N: usize = 3, F: Float = f64> where F: NumAssignOps + ConstOne + AsPrimitive + std::fmt::Debug, @@ -103,6 +103,13 @@ where pub(crate) index: i32, } +impl<'g, P, const N: usize, F> Copy for GridCell<'g, P, N, F> +where + F: Float + NumAssignOps + ConstOne + AsPrimitive + std::fmt::Debug, + P: ParticleLike<[F; N]>, +{ +} + impl<'g, P, const N: usize, F> GridCell<'g, P, N, F> where F: Float + NumAssignOps + ConstOne + AsPrimitive + std::fmt::Debug, @@ -182,23 +189,20 @@ where /// Returns an iterator over all unique pairs of points in this `GridCell`. #[inline] - fn intra_cell_pairs(self) -> impl FusedIterator + Clone { + fn intra_cell_pairs(self) -> impl FusedIterator + Clone { // this is equivalent to // self.iter().copied().tuple_combinations::<(P, P)>() // but faster for our specific case (pairs from slice of `Copy` values) self.iter() - .copied() .enumerate() - .flat_map(move |(n, i)| self.iter().copied().skip(n + 1).map(move |j| (i, j))) + .flat_map(move |(n, i)| self.iter().skip(n + 1).map(move |j| (i, j))) } /// Returns an iterator over all unique pairs of points in this `GridCell` with points of the neighboring cells. #[inline] - fn inter_cell_pairs(self) -> impl FusedIterator + Clone { - self.iter().copied().cartesian_product( - self.neighbors::() - .flat_map(|cell| cell.iter().copied()), - ) + fn inter_cell_pairs(self) -> impl FusedIterator + Clone { + self.iter() + .cartesian_product(self.neighbors::().flat_map(|cell| cell.iter())) } /// Returns an iterator over all _relevant_ pairs of particles within in the neighborhood of this `GridCell`. @@ -208,7 +212,7 @@ where /// This method consumes `self` but `GridCell` implements [`Copy`]. //TODO: handle full-space as well //TODO: document that we're relying on GridCell impl'ing Copy here (so we can safely consume `self`) - pub fn particle_pairs(self) -> impl FusedIterator + Clone { + pub fn particle_pairs(self) -> impl FusedIterator + Clone { self.intra_cell_pairs().chain(self.inter_cell_pairs()) } } diff --git a/src/cellgrid/storage.rs b/src/cellgrid/storage.rs index a418703..a3e06ab 100644 --- a/src/cellgrid/storage.rs +++ b/src/cellgrid/storage.rs @@ -46,7 +46,7 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Default, Clone)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub(crate) struct CellStorage { - buffer: Vec, + pub(crate) buffer: Vec, } impl CellStorage { diff --git a/src/lib.rs b/src/lib.rs index 6070cd7..f556148 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -80,9 +80,17 @@ pub use crate::cellgrid::CellGrid; /// /// This trait is required for types used with [`CellGrid`] /// which needs to know how to get coordinate data.\ -/// Only [`Copy`] types can be used. +/// +///
+/// +/// `ParticleLike` is a subtrait of [`Clone`]. +/// This allows to use the [_interior mutability_](https://doc.rust-lang.org/stable/std/cell/index.html#when-to-choose-interior-mutability) pattern. +/// +/// Usually, [`Copy`] types are preferable (they tend to implement `Clone` by copying). /// In general, the smaller the type, the better (for the CPU cache). /// +///
+/// /// Note that [`CellGrid`] is more specific than this trait and requires implementing `ParticleLike<[{float}; N]>`. /// /// We do not provide a blanket implementation for types implementing `Into<[T; N]> + Copy` but @@ -121,7 +129,7 @@ pub use crate::cellgrid::CellGrid; /// } /// } /// ``` -pub trait ParticleLike: Copy { +pub trait ParticleLike: Clone { /// Returns a copy of this particle's coordinates. fn coords(&self) -> T; } @@ -216,7 +224,7 @@ impl

From

for Particle

{ /// ``` impl ParticleLike<[T; N]> for (L, P) where - L: Copy, + L: Clone, P: ParticleLike<[T; N]>, { #[inline]