Merge the two MinimizeEnergy interfaces

[jngrad]

There are currently two Python interfaces for the steepest descent (SD) algorithm in the core:

• MinimizeEnergy: has type checks, default values, can be checkpointed, no interruption handling
• Integrator: has no type checks, no default values, can be checkpointed, interruption handling

When using Integrator, the previous integrator state is overridden in both the core and interface, and needs to be restored after the minimization ends. When using MinimizeEnergy, the previous integrator state is overridden only in the core, which means running system.integrator.run() afterwards is ill-defined. The structure of class Integrator is also quite heterogeneous and not easily extensible. In particular, the numerous if self._method == X introduce too much coupling between the different types of integrators.

It would make more sense to refactor integrate.pyx with the design of interactions.pyx: an abstract class Integrator from which SD/NPT/VV inherit, and a class CurrentIntegrator (used as System.integrator = CurrentIntegrator()) that contains an instance of Integrator. Adding or removing an integrator algorithm would only be a matter of writing/removing a class.

[RudolfWeeber]

Agreed. Furthermore, the term 'minimize energy' should not appear, since that is not, what it does.

[fweik]

I think if we are considering a (major) interface change here, we should evaluate if the integrator should really be a property of the system. Mixing algorithmic aspects with the definition of the system is not co-aligned with the mental model suggested by statistical mechanics. I've always disliked how the current interface mixes the declaration of what the system is with what you do to it.
If we are willing to break the interface we should put some thought into make it good for once.

To be more verbose: The System class is more incidental than it is designed or thought out. What does it mean? The System in the model sense (e.g. the Hamiltonian or the Liouvillian)? The simulation model? The current state of that model? It has to be clear that those basic elements of the API are what Espresso means to the users, so it is something we should put serious consideration into. It is the language in which people formulate their thesis work in, their research.
I'm by no means an expert user of the software, but I've never heard somebody calling it's interface intuitive, and it stands to reason that this would mean that the interface is not catering to peoples expectations. I've worked with and on the software for more years than I'd care to admit, and still I could not answer really what Espressos terms mean. I know it's easy to dismiss such points as idealistic or esoteric, but I'm convinced that if we try to define concepts that in a way closer to science in our simulation tool we could make even more friends and users.

That being said, I don't think that we should break every other user script again; lack of API stability is the complain I hear the second most often. I think we should think about introducing some API versioning system. E.g. there could be a default, but we could also provide something like from espresso.v4 import System, similar to inline namespace versioning.

[jngrad]

@fweik I mostly agree. But what's the right approach? Re-designing the interface to be meaningful, with a clear separation between physical concepts and algorithms, requires a large effort. Splitting the effort in incremental changes is what seems to work best in the espresso team, because each small change tends to grow rapidly in scope. The present issue is just another sad example of the typical espresso maintenance workflow:

1. Let's merge the two interfaces, this will go fast, I already worked on this class design recently.
2. This went smoothly! Now let's do the same for the other integrators, this is trivial...
3. Wait, why name some integrators NVT and NpT? Those are ensembles, not algorithms.
4. So NVT is just a synonym for velocity Verlet?
5. Wait, velocity Verlet does not exist in the core, typing s.integrator.set_vv() actually does nothing, the old integrator remains active (either NpT or SD). So s.integrator.set_vv() after setting NpT leaves the system in a state where the box still rescales. This is a major bug.
6. But wait, if the previous integrator was SD, s.integrator.set_vv() causes s.integrator.run() to run the integration loop, which silently sets the integrator to NVT because of a side-effect somewhere (it's not from the python interface because s.integrator.set_vv() does not call core functions), so the bug only concerns production runs where NpT is changed to VV.

I don't see how we could maintain multiple APIs without introducing either more complexity or more code duplication, which would both increase the number of bugs. In the present issue, I'd be fine with having set_vv() call set_nvt() for example, which would not break the existing API. However, the MinimizeEnergy API must either go away, or be improved with the same architecture as Integrator (we can then have more than 1 minimization algorithm, although MinimizeEnergy is not the right name; many samples use it to either minimize the force or maximize the distances).

[RudolfWeeber]

So, getting a clear idea of how things do and should inter-relate is something we are working on. I'd probably change the interface, only once we have a clear idea and focus the dis-entangling effort on the core for now.

When we do change the interface at some point, however, it does help if there is exactly one place to touch.

So, short term, I think, only integrator.set_steepest_descent should remain.

[fweik]

What @RudolfWeeber says. I totally agree that this should be changed, but I think we should do one big change when we have figured out that we want do minimize the frequency of breaking interface changes, e.g. such changes that force many users to adapt their scripts.

[jngrad]

The GROMACS team seems to be facing the same issues of integrator maintenance, extensibility and nomenclature as we do:

• cleanup of integration loop
• Proposal for integrator framework (do_md) in future GROMACS
• Roadmap for thermostats / barostats in new propagation/integration scheme
• Rename Integrator
• Model for MD state