Fix #2132

[zemanj]

Fixes #2132

Changes made in this Pull Request:

• _NSGrid.coord2cellid() used to fail for coords close to box boundaries.
  By defining the grid's cellsize as double precision, this is now fixed.
  See Issue capped_distance array: kernel exploding for a specific frame in this trajectory #2132 for details.
• increasing the grid cutoff is now only done when force == False.
  This has never been an issue and is just for consistency.

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[richardjgowers]

Can we include the coordinate from the issue as a test? I don't think we need the actual traj file, we can just hardcode in the box & position that caused this issue

[zemanj]

@richardjgowers Good idea. Could you please verify that this currently fails:

import MDAnalysis as mda
from MDAnalysis.lib import distances
import numpy as np

ref = np.array([[55.783722, 44.190044, -54.16671]], dtype=np.float32)
conf = np.zeros((1, 3), dtype=np.float32)
box = np.array([53.785854, 43.951054, 57.17597, 90., 90., 90.], dtype=np.float32)
pairs, dist = distances.capped_distance(ref, conf, max_cutoff=1.0, box=box, method='nsgrid')

[richardjgowers]

@zemanj that didn't work (ie it worked, therefore didn't work :)), not sure why. This did though:

In [4]: from MDAnalysis.lib import nsgrid

In [5]: fstns = nsgrid.FastNS(3.0, conf, box)

In [6]: fstns
Out[6]: <MDAnalysis.lib.nsgrid.FastNS at 0x10f5f0048>

In [7]: fstns.search(ref)
Segmentation fault: 11

[zemanj]

For me both approaches fail. I'll use the one directly using FastNS with the 3.0 cutoff for testing then.

[codecov]

Codecov Report

Merging #2136 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2136   +/-   ##
========================================
  Coverage    89.43%   89.43%           
========================================
  Files          157      157           
  Lines        18772    18772           
  Branches      2711     2711           
========================================
  Hits         16788    16788           
  Misses        1380     1380           
  Partials       604      604

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca2018d...af46ea7. Read the comment docs.

[richardjgowers]

Looks good, CHANGELOG too

[zemanj]

While we're there: Is there a good reason for FastNS.search() setting up a separate FastNS.searchgrid which is enforced to be identical to the already existing FastNS.grid? I can't think of one.

[zemanj]

@richardjgowers Feel free to squash-merge if you're ok with this.

[richardjgowers]

@zemanj the grids have to match because you iterate over positions in one grid and search in the other grid? It looks like it's used after construction

[richardjgowers]

thanks @zemanj !

[zemanj]

@richardjgowers Of course the search grid has to be identical, and that's exactly what makes it obsolete. It's solely used to determine the search coords' cell ID, which one can simply get from self.grid.coord2cellid(searchcoords_bbox[i]). Anyway, it probably doesn't matter since the overhead cannot be more than O(n).