-
Notifications
You must be signed in to change notification settings - Fork 189
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
Refactored MC displacement move of particles #4599
base: python
Are you sure you want to change the base?
Conversation
7f53fc7
to
d31d611
Compare
d31d611
to
6b34cfa
Compare
How many particles are moved per MC-step? I would make it a parameter of the function. |
@jonaslandsgesell only one particle is moved in each MC step. You can attempt to move as many particles as you want within your sampling cycle by just doing more MC steps: for sample in range(N_samples):
RE.do_particle_displacement_MC_move(mc_steps=N_particles) Optionally, one could add an extra parameter to select how many particles you move in each new configuration before the Metropoli acceptance criteria but I do not see any practical use of it since basically would reduce the efficiency of the MC sampling of any system that I can think of. |
@pm-blanco thanks for your answer :) The more particles are moved per accepted MC step the more the system decorrelates per accepted MC step. I expect a system dependent sweet spot in the number of uncorrelated samples which can be obtained per (realtime) hour. This sweet spot will depend on the force field, the particle density etc. and the a) number of particles to be moved per single MC step and b) the needed compute time per MC step. Since the compute time for the new electric energy computation (in Espresso) is essentially independent from moving one or 10 particles there will be a system dependent sweet spot. The explicit loop over 1-particle MC moves will decorrelate the system slower (i.e. needing more accepted MC steps) while potentially having a higher acceptance rate (measured per MC step). |
@jonaslandsgesell For displacement MC moves, single particle moves are actually more efficient than N particle moves and they are specially recommended for simulations of condensed matter (See https://www.epfl.ch/labs/lcbc/wp-content/uploads/2019/03/MDMC_3.pdf, Section 3.4.1 pages 30 to 33). Of course, depending of the system, the efficiency of the MC conformational sampling can be improved by moving groups of particles using some smart scheme (for instance pivot moves for polymer chains). But this system specific MC moves should have their own function and they are, in my opinion, out of the scope of this general function. |
@pm-blanco the argument presented in the book makes a lot of assumptions for arguing that the random N-particle displacement moves are less computationally efficient but agrees that exact performance has to be benchmarked. To exactly decide if a random N-particle move can be more efficient than N 1-particle moves in Espresso I expect that we need to run benchmarks. Let me make a theoretical example where my expectation is, that running an N particle move is more efficient. I bring this topic up because the PR changes the existing behavior that the code can move multiple particles at once. |
My impression is that this could be modularized more: there should be functions (or maybe functors as they can store parametesr and can be managed via script interface) to
very rough sketch: template <typename ParticleSelector, typename ParticleMoveGenerator>
single_parcile-move(int n_steps, const ParticleSelector& select, ParticleMovegenerator move_gen) {
auto last_energy = calc_potential_energy90
for (int i=0;i <mc_steps; i++)
auto particle_id = select(); // reutrn a std::optioanl<size_t>
if (! particle_id) return false; // no candidates
old_pos = get_particle_position((*particle_id));
auto new_pos = gen_move((*particle_id));
move_particle((*particle_id), new_pos)
if (! within_exclusion_radius(particle_id) {
auto new_energy = calc_potential_energy()
if (metropolis_accept(new_energy-old_energy, kT) {
last_energy = new_energy;
continue
}
}
// else reject and unrolll move
move_particle((*paritcle_id), old_pos);
} Notes:
|
@pm-blanco @jonaslandsgesell In principle, I agree with Jonas that there may be situations where moving multiple particles might be more efficient. However, since I seem to be the only person using the displacement move feature (and I don't need to move multiple particles at once) and the choice was between removing it or fixing it, I would be in favor of leaving it as it is right now. |
Description of changes:
bool ReactionAlgorithm::displacement_move_for_particles_of_type(int type, int n_part)
for a new method to perform MC displacement moves of particlesvoid ReactionAlgorithm::do_particle_displacement_MC_move( int mc_steps, std::vector<int> particle_types_to_move)
mc_steps
as input instead of the number of particles to move, avoiding confusion on the actual number of MC steps performed. If no value is given performs one MC steps by default.particle_types_to_move
, which is the list of particle types from which particles will be selected. If no value is given all particle can be moved by default.ReactionAlgorithm::generate_new_particle_positions(int type, int n_particles)
has been removed since it was only used byReactionAlgorithm::do_particle_displacement_MC_move
m_tried_configurational_MC_moves
andm_accepted_configurational_MC_moves
toN_trial_particle_displacement_MC_moves
andN_accepted_particle_displacement_MC_moves
for clarity.I provide below a minimal working script to test the new implementation.