modernize constraints code - Redmine #2423
This touches several strategic objectives:
- modularize the code better
- use proper C classes
- minimize run-time work passing lots of maybe-unused parameters to do-it-all functions
- minimize development overhead from unrelated changes to types and names of variables that happen to be textually adjacent
- support transition to better auto-tuning
- provide a framework in which a task can be separated from its execution on piece(s) of hardware
- support evolution of integrator framework
- evolve towards the integrator loop building a task graph for subsequent execution, rather than itself being the point of execution
Pieces of work needed in the checklist below, roughly in order of what makes sensible git commits. (NB the checklist will be similar to lists for other code needing modernization - what else should go on this checklist?):
(from redmine: issue id 2423, created on 2018-02-26 by mark.j.abraham)
- Relations:
- relates #2434 (closed)
- parent #1793 (closed)
- Changesets:
- Revision f1ea683f by Mark Abraham on 2018-02-28T14:38:43Z:
Reorganized constraint code
Renamed constraints files after just their algorithm, not the original
programming language, which matches the test file that they
have. Separated header files for each algorithm with matching
names. Removed extern C. Minimized header dependencies. Removed
excessive use of struct keyword.
Removed some header declarations of functions declared static.
Refs #2423
Change-Id: If60ab052ae73ad981fa031c2db6b3e206b380f4c
- Revision 21b21524 by Mark Abraham on 2018-02-28T17:04:32Z:
Moved and removed constraint code
No functionality changes here.
Removed code that was formerly called by the now-removed
combination of Andersen T coupling and constraints.
Removed crattle declaration from header, and moved its definition to
before the point of use, as normal for functions with file static
linkage.
Moved constr_r_max and support functions to constraintrange.cpp,
because conceptually the functionality is independent of which
algorithm implements the constraints. The current implementation only
works with LINCS, but that's because SHAKE and DD can't be used
together.
Refs #2423
Change-Id: I9e4fa1a4bb57118ca17c55f312103439b38190e3
- Revision e573f0b9 by Mark Abraham on 2018-03-13T16:35:25Z:
Add more const to uses of t_commrec and t_inputrec
Also removed some pointless struct keywords.
Refs #2423
Change-Id: Iacc008fb11a31646e798364e253de322c16ccaee
- Revision ad79f778 by Mark Abraham on 2018-04-05T17:28:54Z:
Create gromacs/mdrun module and move code there
Improving how we dispatch the work for do_md and friends requires that
they all compile as part of libgromacs (or all outside of it). The
code currently in src/programs/mdrun is there simply because it hasn't
been moved from the old src/kernel layout, rather than through any
clear design, so we should move it now that we have a reason.
Other code from src/programs/mdrun now needs to move into libgromacs
also, but we currently don't have a module for miscellanous mdrun
features, so we're adding to the abuse of mdlib for now.
Refs #1793
Refs #2423
Change-Id: I4f9ad5d0bf6585675472b49b2d5654f588b7214e
- Revision 1ab524ff by Mark Abraham on 2018-04-12T07:29:30Z:
Reorganize energy evaluation for EM
Used aggregate initialized structs to simplify future refactoring.
Refs #1793, #2423
Change-Id: Iedb8bf3b4cb2d7f238c4b674ce459564a3f76a20
- Revision c8c0605d by Mark Abraham on 2018-04-12T15:57:31Z:
Remove use of where(), if DEBUG etc.
The where() debugging function causes run-time branches in
release-mode builds, and replaces functionality for which one should
use a debugger.
Other such preprocessing clutter should also go away. Since we don't
check whether any of it compiles or produces useful data, we may as
well delete it.
This coincidentally simplifies the call sigatures of some
functionality, too.
Retained some useful essentialdynamics debug code, which is now always
compiled, and written from setup code only at debug level 2.
Refs #2423
Fixes #2122
Change-Id: I2c60c162734f4c2ec1d56153853a36d28e6a66ff
- Revision 15c96888 by Mark Abraham on 2018-04-13T09:58:32Z:
Clean up constraints code
Moved constraint code into gmx namespace.
Used more forward declarations in both the old and new headers.
Removed typedef from type declarations.
Removed gmx_ prefix from some names, since it is no longer required
and we may as well avoid verbiage.
Renamed some types more consistently with newer coding styles, and
called them class now where they will be one shortly.
Added some const correctness.
Apparently this inspires uncrustify to change positions of some
comments. Go figure.
Replaced all gmx_bool in constraints code with bool.
Maths functions from vec.h (which are not in the top-level namespace)
now need namespace qualifications.
Added and fixed various Doxygen, which is primitive in some cases
where the code was particularly cryptic.
Noted policy on multiple authors within Doxygen comments.
Refs #2423
Change-Id: I41bf3a4b9a4fbbcb3a3a7a27dc922d563abedbcb
- Revision acc3940a by Mark Abraham on 2018-04-17T08:03:25Z:
Break apart update_constraints
There are four distinct kinds of work being done, and never was any
call to update_constraints doing all of them, so it's better to have a
group of functions, each of which do one thing, and the relevant ones
called. This also makes it simpler to express by returning fast that
when we don't have constraints, we do nothing.
Made the logic for whether this is a log or energy step match that of
the main MD loop. The old implementation may not have prepared for the
last step correctly when it was triggered by something other than the
nsteps inputrec value.
Removed a commment mentioning iteration, which is a feature
that was removed a while ago.
Removed some ancient debug dump output.
Refs #2423, #1793
Change-Id: I21c10826721ddc9a79a33b1dc75971a20d0855d9
- Revision 5ae5bf42 by Mark Abraham on 2018-04-25T12:12:13Z:
Refactor SD update
The former use of multiple boolean control variables made the logic
hard to follow. Without constraints, the two parts of the integrator
are fused, which is now expressed explicitly.
This means it is now clear that the second half of the sd update does
not compute anything from the foreces.
Removed some of the vestiges of the way we once had two SD
integrators.
Refs #2423, #1793
Change-Id: I39d4cd0b8568859220b3a592b138f3f4405f8991
- Revision e384d530 by Mark Abraham on 2018-06-05T01:05:32Z:
Make Constraints a proper class
Converted the opaque C struct to a pimpl-ed C++ class.
Numerous callers of constraint routines now don't have to pass
parameters that are embedded within the class at setup time,
e.g. for logging, communication, per-atom information,
performance counters.
Some of those parameters have been converted to use const references
per style, which requires various callers and callees to be modified
accordingly. In particular, the mtop utility functions that take const
pointers have been deprecated, and some temporary wrapper functions
used so that we can defer the update of code completely unrelated to
constraints until another time. Similarly, t_commrec is retained as a
pointer, since it also makes sense to change globally.
Made ConstraintVariable an enum class. This generates some compiler
warnings to force us to cover potential bug cases with fatal errors.
Used more complete names for some of the enum members.
Introduced a factory function to continue the design that constr is
nullptr when we're not using constraints.
Added some const correctness where it now became necessary.
Refs #2423
Change-Id: I7a3833489b675f30863ca37c0359cd3e950b5494
- Revision 143ab3d6 by Mark Abraham on 2018-06-05T07:30:45Z:
Separate flexible constraint counting
Functions should do one thing, particularly when the thing is only
needed some of the time and complicates returning multiple things.
Minimized scope of some variables, avoiding risky re-use of variables
with the same name.
Refs #2423
Change-Id: I1860447b4780e76c20a53964a2a946512689b872
- Revision ddc3dd50 by Mark Abraham on 2018-06-13T14:11:38Z:
Use more containers and views for constraints code
Lifetime and usage is clearer if we don't use raw pointers.
Refs #2423
Change-Id: I7d76e30acfd209baf2256efbd3c40290ed38caab
- Revision ec536cfc by Mark Abraham on 2018-06-13T15:41:51Z:
Use initializer lists for Lincs and Task
This permits us to have some constructors that work the same way that
snew used to do (ie. calloc), so that future cleanup can use more
vectors, etc.
Also made a destructor of Constraints::Impl so the Lincs object gets
freed.
Refs #2423
Change-Id: Ie4a2001556d04f620a9d5241a9e1da9024cb9ce1
- Revision dd0560e4 by Mark Abraham on 2018-06-13T22:18:24Z:
Call Constraints::setConstraints with other setup code
Refs #2423
Change-Id: Iebee644a133f60778fea9b8030b7b7bf9656ba82