Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split thermostats into separate classes #3980

Closed
jngrad opened this issue Oct 31, 2020 · 0 comments · Fixed by #4845
Closed

Split thermostats into separate classes #3980

jngrad opened this issue Oct 31, 2020 · 0 comments · Fixed by #4845
Labels

Comments

@jngrad
Copy link
Member

jngrad commented Oct 31, 2020

There is currently a single Thermostat class in the python interface that mixes the LB, NpT, DPD, Langevin, Brownian and Stokesian thermostats. This design is difficult to maintain, especially in the checkpointing methods where all thermostat internals are visible in the same scope.

The Integrate class was in a similar situation in 4.1, and was split up into individual classes in 4.2 without breaking backward compatibility (#3390). We could apply the same refactoring to the Thermostat class. The difference with the Integrator class is that multiple thermostats can be active simultaneously. In the new design, Thermostat would store a list of currently active thermostats.

This new design would allow us to inactivate a single thermostat, as we do with Actors. At the moment, to inactivate a thermostat we have to inactivate all thermostats and re-enable those we didn't intend to inactivate, which means keeping their original parameters in a variable and setting them up again, in the same order, because each thermostat overwrites the global temperature variable.

@jngrad jngrad added the Python label Oct 31, 2020
@kodiakhq kodiakhq bot closed this as completed in #4845 Jan 19, 2024
kodiakhq bot added a commit that referenced this issue Jan 19, 2024
Description of changes:
- encapsulate thermostats and remove their Cython interface
   - use one C++ `ScriptInterface` class per core class (fixes #3980)
   - introduce the `ThermostatFlags` enum, allow deactivating a single thermostat, add an `is_active` flag accessible from the Python interface (fixes #4266)
- remove 20 MPI callbacks
   - partial fix for #4614
- remove thermostat and thermalized/rigid bond global variables
   - partial fix for #2628
- remove a significant amount of historical Cython code that relied on deprecated Cython features
   - partial fix for #4542
   - ESPResSo is now compatible with Cython 3.0
   - the `nogil` deprecation warning subsists
- improve testing of the thermostats interface
- API changes:
   - thermalized bond parameter `seed` was moved to `system.thermostat.set_thermalized_bond()`
      - this change better reflects the fact there is only one global seed for all thermalized bonds
      - until now this global seed was overwritten by any newly created thermalized bond, whether it was added to the system or not
   - LB thermostat `seed` parameter now gets written to the RNG seed (silent change)
      - until now, the RNG seed was hardcoded as `0` and the `seed` value was used to set the RNG counter, thus two independent simulations with a different seed would actually get the same particle coupling random noise sequence with a time lag equal to the difference between the two seeds
      -  this is a bugfix for ensemble runs
      - fixes #4848
@github-project-automation github-project-automation bot moved this from Todo to Done in Propagation refactoring Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging a pull request may close this issue.

1 participant