Skip to content

Use builders to create modular simulator elements

Pascal Merz requested to merge ptmerz-builders into master

The current setup of modular simulator mixes constructing the elements, satisfying their mutual connections (registering clients to signallers and functionality providers), and their arrangement (which defines the exact integration algorithm being used). This requires building and linking the elements in a implicitly defined way. This is error-prone, harms extensibility, and obfuscates much of the simplicity of the modular simulator approach.

The new approach is explained in docs/doxygen/lib/modularsimulator.md, "### Building modules" (!96 (diffs)). In summary, it divides building the modular simulator in three phases:

  • construct the element builders (in any order),
  • connect the element builders (in any order),
  • use builders to build elements and put them in order of execution, effectively defining the integrator algorithm.

The best example of the improvements is probably the buildIntegrator function in src/gromacs/modularsimulator/modularsimulator.cpp, which changed from a very messy mixture of construction, initalization and deployment to a function that makes it intuitively understandable what algorithm is implemented.

How to review this MR:

Note that this MR is intended to be merged without squashing its commits. It is a trial run for a type of feature branch merge model. The current MR is very large (38 files, +2896 -927). It is, however, limited in scope (except for an added exception, all changes are inside src/gromacs/modularsimulator/) and highly repetitive. If there is agreement on the general design (outlined in the first few commits), it is significantly more efficient for the reviewer to review all commits at once than to stage them sequentially as independent MRs. Keeping the commits sequential in master will also make later bisecting easier. Squashing all commits, however, would make later bisecting impossible. We will therefor try the following approach:

  • Merge this MR without squashing
  • Require that every commit passes the full CI
  • Use rebasing and git push --force to keep the commit list clean
  • Use live review sessions (via Zoom) lead by the committing developer to help reviewers
  • Document all feedback and decisions in this MR to allow developers not attending the live review sessions to voice their opinion

Note that this is an experiment for an alternative way of reviewing changes, so feedback on the process is as welcome as feedback on the code!

Closes #3437 (closed)

Edited by Pascal Merz

Merge request reports