espressomd / espresso

The ESPResSo package
https://espressomd.org
GNU General Public License v3.0
230 stars 188 forks source link

LB/Virtual sites: incomplete API change #4935

Closed RudolfWeeber closed 4 months ago

RudolfWeeber commented 5 months ago

With the propagatoin refactor, the coupling fo virtual sites to an LB fluid is now controlled on the particle level by setting the appropriate propagation flag or when constructing the virtual site with p.vs_relate_to().

So

system.thermostat.set_lb(..., act_on_virtual=True)

has no effect. However, this keyword argument is still silently accepted.

To resolve:

jngrad commented 5 months ago

There is no act_on_virtual anywhere in the source code or user guide, so we cannot remove it. The way our script interface was designed, unrecognized arguments passed in calls to class methods are always silently ignored. Introducing checks on argument names would be quite a challenge, because we leverage the flexibility of the interface to inject secret arguments (with a leading underscore symbol) to make checkpointing possible, and simplify the testsuite for some features. Sometimes, we allow two different sets of argument names, depending on whether the use wants to e.g. set a relative virtual site from a particle, or from a particle id.

As far as I know, the user guide is up-to-date w.r.t. virtual sites. We did not document the propagation flags, but we could add a section after virtual sites in particles.rst, such that the table of contents looks like this:

5.8. Rotational degrees of freedom and particle anisotropy
5.9. Virtual sites
5.10 Propagation flags