Refactor thermostat infrastructure #3888
Conversation
In the previous implementation, thermostat noise for different seeds was correlated, the seed just shifted the sequence. For example with seed offset X, the cross-correlation used to have a peak at lag X. Co-authored-by: Jean-Noël Grad <[email protected]>
The SD external library uses Philox internally for both the CPU and GPU implementations. SD now uses the global counter instead of the simulation time divided by the time step, which are both mutable and can lead to correlated sequences when changed by the user.
Remove overhead from the std::function wrapper and heap allocation.
Reduce sample sizes and increase tolerances.
Header file thermostat.hpp no longer includes integrate.hpp.
Use vector operations, expressive variable names, documented macros.
Call the thermostat methods directly. We only need free functions for MPI callbacks.
jngrad
force-pushed
the
thermostats-correlation
branch
from
b81ebc2 to
5428fca
Compare
|
I think we already discussed that we do not want to have one counter per thermostat. Let's please first discuss and then implement. |
|
You can view this PR draft as a subset of the first one. It's unclear to me how the DPD specifics can be resolved and how much time it will take. This PR solves all correlations issues; moving from the state in this PR to the state with a global counter should be straightforward. |
|
For every case possible, the counter increment should be in one central place called from the integrate function, shouldn’t it?
That would already give us some of the benefit (and the basic structure) for the global counter.
For now, dpd with constraints has to be the exception.
|
Yes, that's what is achieved by this PR: the integration loop in |
jngrad
force-pushed
the
thermostats-correlation
branch
from
3115d3d to
d21c7a4
Compare
Description of changes:
Systemsim_timeortime_stepduring simulations with SD (fixes Noise behavior of stokesian dynamics and other thermostats differ #3840)