Improve analysis neighborhood search API
NeighborhoodSearch class in source:src/gromacs/trajectoryanalysis/nbsearch.h is a very thin wrapper around original C code in source:src/gromacs/selection/nbsearch.h. It would be best to just convert the C code into C++, improving the interface in the process, and get rid of the wrapper. Would be good to add unit tests at the same time to make sure that the converted code works.
Fix more Doxygen warnings.
- Add documentation for some partially documented classes.
- Some fixes to cosmetic issues in surrounding code that were noticed.
- Some other clean-up.
There are a few warnings in pre-5.0 C code that are not addressed here.
Of the remaining warnings, quite a few are are either in obsolete code
(template.c, displacement.c), or in code that should be heavily
refactored (nbsearch.h, issue #866).
Merge two separate nbsearch.h files.
Very small part of IssueID #866
Proper C++ API for analysis neighborhood search.
Replace the dummy C++ wrapper with a more C++-like API for the
neighborhood search routines that are part of the selection code.
Subsequent commits will implement a few additional features to the API,
but split those off from here to keep things easier to review.
Implementation underneath is still the same as earlier.
Will remove the old C functions in a separate commit.
Changed the free volume tool to use the C++ API in preparation for this.
Added some basic tests for the functionality using the new API, which
should increase the percentual coverage in this part of the code
Better concurrency support for analysis nbsearch.
AnalysisNeighborhood::initSearch() now supports concurrent calls from
multiple threads such that each call creates an independent search.
A pool of search objects is kept allocated to avoid constant
reallocation of the data structures.
This allows using the neighborhood search routines transparently in
thread-parallel analysis tools after that is implemented in the
framework. The changes in the template demonstrate how this simplifies
the tool code.
Part of #866.
Remove C API for analysis neighborhoor search.
Makes it easier to refactor the C++ API, when no consideration needs to
be put into maintaining the C interface. If really necessary, this can
be reimplemented on top of the C++ API later.
Nearly the whole nbsearch.cpp file changes, but most of this is
mechanical replacement. Kept the order in the code as same as possible
to make it easier to see that the logic is not changing.
The high-level changes are:
- Move members of gmx_ana_nbsearch_t to the appropriate implementation
classes and suffix them with an underscore.
- Move static helper functions into members of the appropriate
implementation classes. This and the above are responsible for
majority of the changes, as 'd->member' becomes 'member_' on most
- Move the declarations of the C++ implementation classes to the
beginning of the file.
- Move all Doxygen documentation to the classes.
- Restructure the C++ classes a bit to remove some workarounds that
were necessary to use the C API underneath. Most notably, the forced
search mode is now passed directly to the initialization function,
removing some extra boolean flags, and the const methods in
AnalysisNeighborhoodPairSearch are now really const.
Also some minor refactoring included here:
- Remove use of cbrt(), and thus dependency on config.h.
- Some conversion to C++ patterns: use of static_cast and moving
variable declarations close to the first use.
Will continue the clean-up in subsequent commits to make it easier to
review that this part really does not change the old functionality.
Part of #866.
C++-ify analysis nbsearch more.
- Use static_cast instead of C casts throughout.
- Use more C++-style comments.
- Use the same pattern for startPairSearch() as for initSearch() to
allow multiple concurrent searches.
- Use std::vector instead of explicit memory allocation for the grid
cell management. Memory is still managed explicitly for rvec and ivec
arrays, since they can't be put into std::vector.
- Make most members of the implementation classes private to clarify the
code (the implementation class is now explicitly responsible for
managing its internal state).
Part of #866.
More flexible position input for nbsearch routines.
In addition to single positions, it is now possible to do a full
pairwise search between two set of positions. Introduced a helper class
that encapsulates all information about the positions, such that all
methods can take this class as input, and all different forms of
initializing the positions only needs to be implemented once in this
class. Also made it possible to pass selections directly to the
neighborhood search routines and removed deprecated
Selection::positions(), which was only useful in combination with the
Part of #866.
#2 Updated by Teemu Murtola over 7 years ago
- Status changed from New to In Progress
- Assignee set to Teemu Murtola
- Target version changed from future to 5.0
Decided to implement this to the end in an effort to reduce the number of stuff I have in progress. The main motivation is to get better test coverage for this part of the code, and those tests are easier to implement against a proper C++ interface (and the interface needs some additions to make it easy to test the different paths). But as said earlier, a proper C++ interface is also nicer to have for writing new code.