Original report by Matthew Spellings (Bitbucket: mspells, GitHub: klarh).

Currently, CorrelationFunction always sets the correlation function value at the first bin to be 0 (really, the value for the default constructor). This is acceptable if you're computing the self-correlation of one set to itself and don't want the first value, but if you're computing cross-correlation between two different sets, it is not necessarily what you want.

Consider the following code, which computes the correlation of one point at the origin (with associated value 1) to a randomly-generated set of points with associated values of the magnitude of their radial distance from the origin. The correlation function should then be identity.

import numpy as np
import numpy.testing as npt
from freud import trajectory, density

ref = np.array([[0, 0, 0]], dtype=np.float32)
refVals = np.array([1], dtype=np.float32)
rmax = 10.0
dr = 1.0
num_points = 10000
box_size = rmax*3.1
points = np.random.random_sample((num_points,3)).astype(np.float32)*box_size - box_size/2
pointRs = np.sqrt(np.sum(points**2, axis=-1))

cf = density.FloatCF(trajectory.Box(box_size), rmax, dr)
cf.compute(ref, refVals, points, pointRs)

import matplotlib, matplotlib.pyplot as pp
pp.plot(cf.getR(), cf.getRDF())
pp.show(block=True)

Currently, however, master sets the value at the first bin to be 0 unconditionally.

Description

EnvironmentMotifMatch constructs an environment for each particle and matches it to the environment of a reference particle (the motif). The size of the local environment for each particle may, however, be larger than the motif size, since any subset of that environment may be sufficient to match the motif. Therefore, the m_max_num_neigh parameter cannot be set to the size of the motif, but must instead be computed dynamically from the NeighborList.

The underlying bug looks like it has always been present, but prior to freud 2.0 it was much harder to encounter because it would require a user to manually construct a NeighborList and then pass a value of k (the number of neighbors) to the constructor of the MatchEnv object that did not match the one used for constructing the NeighborList. The default constructed NeighborList within MatchEnv would always match the value of k correctly. With freud 2.0, it became much easier for users to specify alternative neighbor specifications with the new query syntax, and the value of k was no longer part of the class definition. #489 introduced the specific error that made it easy to hit this error case by always setting the value of EnvDisjointSet.m_max_num_neigh to the size of the motif, so any neighbor specification resulting in particles with more neighbors in the NeighborList than the size of the motif would trigger this error.

Motivation and Context

Resolves: #633

How Has This Been Tested?

Both the original script in #633 (with the data included there) and the example documented by @Charlottez112 on #978 seg fault on my machine without this change, and both of them complete successfully once these changes are included.

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds or improves functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

• [x] I have read the CONTRIBUTING document.
• [x] My code follows the code style of this project.
• [ ] I have updated the documentation (if relevant).
• [x] I have added tests that cover my changes (if relevant).
• [x] All new and existing tests passed.
• [ ] I have updated the credits.
• [x] I have updated the Changelog.

From roadmap planning session with @vyasr and @bdice. To be assigned after #176 is closed.

Standardize the freud API

Once #176 (doc cleaning) is done, we should identify what we want APIs to look like for everything in freud.

1. Review existing APIs
2. Determine a standard for all modules to follow, with minimal exceptions
3. Create a list of cases where the standard is not currently followed

The behavior of compute/accumulate/property getters is one notable case that should be standardized.

For methods that we want to remove (e.g. getRDF, which should be replaced by a property), we will remove that method from the documentation and add deprecation warnings for version 2.0.

For any class/function where the signature has to change, we will make use of *args, **kwargs to take variable APIs and then dynamically resolve them. Deprecation warnings will be issued wherever appropriate.

Description

I am dealing with an unusual case with Voronoi neighbors in a sparse system (only a few particles), where particles can be their own neighbors or share multiple bonds with the same neighbor. For this case, it's insufficient to identify neighbors by their distance. Instead, I need the actual vectors computed by the Voronoi tessellation.

• Make vec3, vec2, quat constexpr-qualified (also constexpr implies inline).
• Add vectors to NeighborBond, NeighborList.
• Store neighbor vectors in all neighbor bonds, use this data instead of re-computing bond vectors from the query points and points.
• Ignore E402 because flake8 doesn't like some changes made by isort; I prefer isort so I'm ignoring the flake8 warning.

There is an API break worth mentioning here: NeighborList.from_arrays can no longer accept distances, and instead requires vectors. (The distances are computed internally to match the vectors.) I could not think of a way to avoid this API break.

Motivation and Context

In this PR, I re-work the NeighborBond and related classes to store the actual vector for each bond. This vector can be directly re-used in many analysis methods, avoiding the need to perform computing box.wrap(points[point_index] - query_points[query_point_index]). I have benchmarks below. This should help improve correctness in a few edge cases like the Voronoi case I mentioned above, and should make it easier if we ever choose to let freud find neighbors beyond the nearest periodic image. I also caught a bug in the tests because of this change.

How Has This Been Tested?

Existing tests pass with minor tweaks.

Performance:

• RDF is basically unchanged
• BondOrder is ~10% faster
• PMFT is ~10% slower

Overall I'm not concerned about the performance impact of this PR.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds or improves functionality)
• [x] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

• [x] I have read the CONTRIBUTING document.
• [x] My code follows the code style of this project.
• [x] I have updated the documentation (if relevant).
• [x] I have added tests that cover my changes (if relevant).
• [x] All new and existing tests passed.
• [ ] I have updated the credits.
• [ ] I have updated the Changelog.

Original report by Carl Simon Adorf (Bitbucket: csadorf, GitHub: csadorf).

I'm currently porting my scripts to python3.4 on collins when I encountered this bug.

The bug occurs when I try to calculate the RDF from a previously read XMLDCDTrajectory.

• freud version: c02760af62e9482d58222deb158770ee05ce3368
• hoomd version: HOOMD-blue v1.0.1 CUDA DOUBLE MPI SSE AVX
• python: 3.4.1

#!python

Boost.Python.ArgumentError: Python argument types in
    RDF.__init__(RDF, Box, float, float)
did not match C++ signature:
    __init__(_object*, float, float)
[collins:25249] *** Process received signal ***
[collins:25249] Signal: Segmentation fault (11)
[collins:25249] Signal code: Address not mapped (1)
[collins:25249] Failing at address: 0xa0
[collins:25249] [ 0] /lib64/libpthread.so.0(+0x10e50) [0x7f6d427f4e50]
[collins:25249] [ 1] /usr/lib64/libpython3.4.so.1.0(+0x9bc38) [0x7f6d43c2cc38]
[collins:25249] [ 2] /usr/lib64/libpython3.4.so.1.0(+0xa5897) [0x7f6d43c36897]
[collins:25249] [ 3] /usr/lib64/libpython3.4.so.1.0(+0xa5297) [0x7f6d43c36297]
[collins:25249] [ 4] /lib64/libc.so.6(__cxa_finalize+0x97) [0x7f6d3de4b327]
[collins:25249] [ 5] /usr/lib64/libboost_python-3.4.so.1.55.0(+0x17743) [0x7f6d42e8a743]
[collins:25249] *** End of error message ***
Segmentation fault

Description

Freud's CMake used the TBB_INCLUDE_DIR and TBB_LIBRARY variables, instead of just linking to the TBB build target, which caused build issues on #866 . This PR makes changes to fix the build issues on certain systems and use better modern CMake style.

Motivation and Context

Resolves: #866

How Has This Been Tested?

The CI will test the new CMake code on many different systems, and I will verify that the system configuration referenced in #866 builds freud correctly.

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds or improves functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

• [ ] I have read the CONTRIBUTING document.
• [ ] My code follows the code style of this project.
• [ ] I have updated the documentation (if relevant).
• [ ] I have added tests that cover my changes (if relevant).
• [ ] All new and existing tests passed.
• [ ] I have updated the credits.
• [ ] I have updated the Changelog.

Inplace argument added in box.wrap

Description

Added inplace argument in both box.wrap and util._convert_array

Motivation and Context

Resolves: Reduce copying of input data and add an option to operate on the data directly.

How Has This Been Tested?

Added some unit tests in test_box_Box.py and test_util.py, and passed these tests.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds or improves functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

• [x] I have read the CONTRIBUTING document.
• [x] My code follows the code style of this project.
• [x] I have updated the documentation (if relevant).
• [x] I have added tests that cover my changes (if relevant).
• [x] All new and existing tests passed.
• [x] I have updated the credits.
• [x] I have updated the Changelog.

Description

Use scikit-build and CMake to build the C++ and Cython components of freud.

Motivation and Context

This pull request dramatically speeds up builds of freud (takes < 30 seconds on my Mac using Ninja). Changing to a proper build system should also dramatically simplify addressing issues like #464 and #629. It will also enable easier integration with IDEs for the C++ code, and long term it will enable exposing a C++ API for freud.

Outstanding tasks:

• [x] Enable automatic download of submodules.
• [x] Contact voro++ maintainer to discuss modifications that would allow using it as a library (rather than having to handle compiling its sources ourselves).
• [x] Update CI scripts to work with new build system.
• [x] Update and test deployment (to Test PyPI).
• [x] Update development requirements.
• [x] Update documentation on building freud.

Update -- The comments below are mostly outdated now due to later changes to the CMake configuration, but I'll leave them here so that we can track the discussions in the future if needed.

This PR supersedes #661. I glanced through that PR, and I note that this implementation is different: rather than compiling a single shared object for the entire C++ library, I am instead following a more distributed model where each Cython module is compiled against the corresponding C++ code. We can change this in the future if we move towards a more thorough separation of the C++ code and exposing those APIs, but for now I'd prefer to retain this model since it's cleaner and generates leaner builds.

@joaander as a more experienced CMake user, some specific questions for you (in addition to any review you can provide).

• What is a reasonable minimum version of CMake to use? I've looked through the changelog etc briefly, but I'd prefer a more informed opinion.
• If you look at freud/CMakelists.txt, I'm forced to link the _util Object library directly to a couple of Cython modules rather than to the corresponding C++ Object libraries (i.e. _environment instead of environment). I think that the reason is that the relevant C++ util code (the diagonalize.cc source file) is only included by the source files of the corresponding C++ modules, not the headers. As a result, I've marked the corresponding target includes (for instance, in cpp/environment/CMakeLists.txt) as PRIVATE; however, I note that changing this to PUBLIC doesn't fix the issue, so I'm not certain this is correct. It may also be some limitation with Object library includes propagation that I'm not finding in the documentation. Any thoughts?
• I wrote a simple FindTBB.cmake script, but I know that we discussed the more sophisticated version along with the config file that you have on HOOMD's next branch. Would you recommend I try to copy in that file and the corresponding macro?

How Has This Been Tested?

Existing builds on my machine are functional. Additional testing will come in the form of the to-do list above.

Update - Building and testing on CI now works on all systems. The wheel building process has also been manually verified via push to test PyPI.

Screenshots

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds or improves functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

• [x] I have read the CONTRIBUTING document.
• [x] My code follows the code style of this project.
• [x] I have updated the documentation (if relevant).
• [x] I have added tests that cover my changes (if relevant).
• [x] All new and existing tests passed.
• [x] I have updated the credits.
• [x] I have updated the Changelog.

@Plastikschuessel noted that the documentation for the Hexatic order parameter doesn't match the implementation. The docs say that the formula normalizes by 1/n, but the implemented code normalizes by 1/k. In many systems (particularly hexagonal and square crystals), k=n, but in some cases (e.g. 2D quasicrystals), the order is 12-fold even though any given particle has fewer than 12 neighbors.

https://github.com/glotzerlab/freud/blob/3f44951a7eb8d6670664fc1dac22c451f17278a1/freud/order.pyx#L250-L251

https://github.com/glotzerlab/freud/blob/3f44951a7eb8d6670664fc1dac22c451f17278a1/cpp/order/HexaticTranslational.cc#L38-L45

A temporary workaround to achieve the desired behavior is to enabled weighted=True in the constructor. In ball queries and nearest neighbor queries (but not Voronoi queries), the weight for each bond defaults to 1. If weighted=True, the normalization will divide by the number of neighbor bonds (a sum with 1 for each bond).

To resolve this issue, we should verify the literature to see which convention is more common and consider changing to normalization by 1/n instead of 1/k.

Hi,

I'm wondering if it's feasible to bound the 3D space by a dilatation of the convex hull of a set of points instead of a cube?

Indeed, for my problem I have points at the top and bottom surface which are not aligned on two planes. This creates quite big voronoi cells at the top and/or bottom (see an example in 2D bellow)

Also I noticed on the example above that cells at the top have a high number of sides, does that mean that they are considered to be neighbors to a lof of other top cells, even if not shown in the plot?

Thanks a lot for your help and congratulations on the repository!

Description

A commonly desired quantity from simulations is S(q), the static structure factor. The recently introduced diffraction module (#596) is an ideal location for this feature. In my understanding, the structure factor can be calculated two different ways: directly (which is expensive, O(N^2) for a system of N particles), or indirectly via a Fourier transform of a radial distribution function. I have heard there is significant controversy over the choice of method and the regimes in which each is correct. I hope to offer both methods in this pull request, as well as some clarity in the documentation for when each might be (in)appropriate for use.

The second thing to outline is the scope of this pull request. This is a first-pass, and will only support systems with a single species, and only one set of particle positions (i.e. points == query_points). This PR is intentionally held to a very narrow scope, to reduce the complexity of the initial implementation and to solidify testing requirements for the "base case" upon which further features may someday be added, such as:

• multi-species structure factors (via points and query_points as well as hints on how to normalize the result correctly)
• accumulation over multiple frames (using freud's standard reset=False approach -- but may require additional normalizations!)
• particle form factors (e.g. via a secondary array of values)
• bonded contributions e.g. from polymers (I think there might be another term for this, but not sure)

Motivation and Context

Discussed with @ramanishsingh and also desired for my own research.

The reason to implement this feature in freud (rather than point users to another existing package) is that it complements and can leverage the fast neighbor-finding and other features of freud, the feature itself can be implemented in parallelized C++, and it fits in the scope of colloidal-scale simulation analysis that freud emphasizes.

Resolves: #652

TODO (help welcome)

• [x] Consolidate implementation code from Cython into C++
• [x] Validate results approximately match against a known reference
• [ ] Validate FFT-RDF method against direct method for q values greater than 4pi/L (or something like that) where they should agree
• [ ] Write docs and seek expert knowledge (literature) about when each method is valid
• [ ] Write tests to ensure behavior is not broken if/when new features are added

How Has This Been Tested?

I plan to compare this code against a few existing implementations to verify its accuracy.

Validate against:

• https://github.com/mattwthompson/scattering/
• other implementations from Glotzer group members

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds or improves functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

• [x] I have read the CONTRIBUTING document.
• [x] My code follows the code style of this project.
• [x] I have updated the documentation (if relevant).
• [x] I have added tests that cover my changes (if relevant).
• [x] All new and existing tests passed.
• [x] I have updated the credits.
• [x] I have updated the Changelog.

Bumps numpy from 1.23.5 to 1.24.1.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Description

This PR adds a new concept in freud, the NeighborList filter, and the SANN neighbor finding method implemented as a neighborlist filter. The SANN method finds neighbors based on solid angle occupied by each neighbor up to a total of 4pi. For more information, see https://aip.scitation.org/doi/10.1063/1.4729313

I have also added a method for sorting a NeighborList by either distance or point_index to the python API. It was needed for the SANN method implementation and I think it has general utility as well.

Motivation and Context

This is a useful way of finding neighbors, which was not previously available in freud.

How Has This Been Tested?

Tests have been added in test_locality_Filter.py. I would like to see some validation of the calculation on a real system before this PR can be merged.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds or improves functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

• [ ] I have read the CONTRIBUTING document.
• [ ] My code follows the code style of this project.
• [ ] I have updated the documentation (if relevant).
• [ ] I have added tests that cover my changes (if relevant).
• [ ] All new and existing tests passed.
• [ ] I have updated the credits.
• [ ] I have updated the Changelog.

Description

Please assist me in improving the code @tommy-waltmann To calculate time-dependent intermediate scattering function. Here are the refs:

1. https://en.wikipedia.org/wiki/Dynamic_structure_factor
2. https://www.lehigh.edu/imi/teched/AtModel/Lecture_11_Micoulaut_Atomistics_Glass_Course.pdf

Motivation and Context

Here we add a class derived from StaticStructureFactorDirect to calculate the time-dependent intermediate scattering function. Resolves: #1040

How Has This Been Tested?

This code has not been tested before complete

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ x] New feature (non-breaking change which adds or improves functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

• [ x] I have read the CONTRIBUTING document.
• [x ] My code follows the code style of this project.
• [ ] I have updated the documentation (if relevant).
• [ ] I have added tests that cover my changes (if relevant).
• [ ] All new and existing tests passed.
• [ ] I have updated the credits.
• [ ] I have updated the Changelog.

Description

The intermediate scattering function is defined as the Fourier transform of the Van Hove function: Instead of Fourier transform, these functions can also be directly computed from the atomic trajectories: Fs and Fd are self and distinct parts.

Is there any plan to support this function? Otherwise, I can write one following structure factor code and MSD part.

Proposed Solution

isf = freud.Scattering.Intermedidate(k_space, ...)
# points = (N_frames, N_particles, 3)
# points = (N_frames, M_particles, 3)
isf.compute((box, points)).query(query_points)
isf.self_part
isf.distinct_part

Additional Context

Reference: https://www.lehigh.edu/imi/teched/AtModel/Lecture_11_Micoulaut_Atomistics_Glass_Course.pdf

Developer

Would someone else please implement this?

Description

This PR removes the global_search flag in EnvironmentCluster, in favor of having users just give a neighborlist where every particle is a neighbor of every other particle.

Motivation and Context

This PR makes the API more intuitive and less confusing.

Resolves: #984

How Has This Been Tested?

I have converted a previous test that used the global_search=True option.

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds or improves functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

• [ ] I have read the CONTRIBUTING document.
• [ ] My code follows the code style of this project.
• [ ] I have updated the documentation (if relevant).
• [ ] I have added tests that cover my changes (if relevant).
• [ ] All new and existing tests passed.
• [ ] I have updated the credits.
• [ ] I have updated the Changelog.

Description

The environment.LocalDescriptors documentation doesn't really give a description of what the local descriptors are, how they are calculated, or even a reference that explains what they are. As far as I can tell, one would have to actually look through the source code to find out what this class actually calculates. In a previous version of the documentation there was this example but it is no longer in the 'stable' branch. The documentation for this class should be more explicit about what it calculates and what its properties are.

Motivation and Context

As an end user, it is hard to know whether and how to use classes like this without more information about what they are calculating.