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

WIP: nearest N particle search #3382

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

Conversation

richardjgowers
Copy link
Member

Fixes #

Changes made in this Pull Request:

PR Checklist

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

@richardjgowers
Copy link
Member Author

@hmacdope I think if you get your head round how the results container is keeping track of the "best" N results, then how the cubeiter works the actual search function isn't that nasty tbh. It's currently failing because of a double free somewhere, so I think I've got my cython wrong somewhere but I'll track that down.

@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #3382 (a98e29d) into develop (8f9f402) will increase coverage by 0.01%.
The diff coverage is 96.73%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3382      +/-   ##
===========================================
+ Coverage    93.62%   93.63%   +0.01%     
===========================================
  Files          176      176              
  Lines        22839    22915      +76     
  Branches      3225     3227       +2     
===========================================
+ Hits         21382    21456      +74     
- Misses        1406     1408       +2     
  Partials        51       51              
Impacted Files Coverage Δ
package/MDAnalysis/lib/distances.py 97.83% <77.77%> (-0.58%) ⬇️
package/MDAnalysis/lib/nsgrid.pyx 97.88% <98.79%> (+0.23%) ⬆️
package/MDAnalysis/coordinates/PQR.py 95.23% <0.00%> (+0.91%) ⬆️

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 8f9f402...a98e29d. Read the comment docs.

@pep8speaks
Copy link

Hello @richardjgowers! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 1393:1: W293 blank line contains whitespace
Line 1400:80: E501 line too long (88 > 79 characters)
Line 1405:80: E501 line too long (94 > 79 characters)
Line 1409:1: W391 blank line at end of file

@cbouy
Copy link
Member

cbouy commented Jan 12, 2022

I'm a bit late to the party but doesn't the KDTree query from scipy already do what you need?

@richardjgowers
Copy link
Member Author

richardjgowers commented Jan 12, 2022 via email

grid_spacing = 4.0

if box is None:
raise ValueError("TODO")
Copy link
Member

Choose a reason for hiding this comment

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

use KDTree as suggested by @cbouy ?

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

Successfully merging this pull request may close these issues.

None yet

5 participants