Project

General

Profile

Bug #1117

Feature #1292: mdrun features to deprecate for 5.0

Feature #1500: Post-5.0 feature clean-up plan

ensemble-averaged distance restraints is probably broken

Added by Mark Abraham almost 7 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Low
Assignee:
-
Category:
-
Target version:
Affected version - extra info:
Affected version:
Difficulty:
uncategorized
Close

Description

This feature predates a lot of parallelism implementations and (IIRC) the REMD implementation (with which it shares the use of -multi), so the assumptions under which it was written are probably broken by now. I suspect, but cannot test, that it does not work with more than one process per simulation.

If this isn't important functionality for anyone, it might not make the cut for 5.0


Related issues

Related to GROMACS - Task #1971: Removing buggy features vs. keeping workflows New
Related to GROMACS - Bug #1989: simple distance restraints should work with REMD and multiple ranks per simulationClosed
Related to GROMACS - Bug #408: unwanted ensemble averaging of distance restraints with multisim / replica exchangeClosed05/01/2010
Related to GROMACS - Bug #2029: mdrun gives different distance restraint potential energies depending on the number of openMP threadsClosed

Associated revisions

Revision 64600020 (diff)
Added by Mark Abraham almost 7 years ago

Updated code checks with distance restraints

Ensemble-averaged distance restraints require -multi, and it is
also reasonable to do REMD with distance restraints, which also
requires -multi. So checks for the ensemble-averaging case need
to be more sensitive to their context.

In fact, ensemble-averaging is probably functional only with PD and
one processor per system, since that was probably all that was
available when it was built.

Fixes #613, refs #1117

Change-Id: Ia6f1bb4eb82eab3c7c249638cd3a5a5d1f707132

Revision c1364cf4 (diff)
Added by Berk Hess about 3 years ago

Made distance restraints work with threads and DD

The NMR distance restraints use several buffers for summing distances
that were indexed based on the index of the thread+domain local ilist
force atoms. This gives incorrect results with OpenMP and/or domain
decomposition. Using the type index for the restraint and a domain-
local, but not thread-local index for the pair resolves these issues.
The are now only two limitations left:
  • Time-averaged restraint don't work with DD.
  • Multiple copies of molecules in the same system without ensemble
    averaging does not work with DD.

Fixes #1117.
Fixes #1989.
Fixes #2029.

Change-Id: Ic51230aa19a4640caca29a7d7ff471e30a3d9f09

History

#1 Updated by Mark Abraham over 6 years ago

  • Target version changed from future to 5.0
  • Affected version set to 4.6.1

That is, might get axed for 5.0

#2 Updated by Mark Abraham over 5 years ago

  • Target version changed from 5.0 to 5.x
  • Parent task set to #1500

#3 Updated by Mark Abraham over 5 years ago

  • Description updated (diff)

Unless someone is able to give me an ensemble-averaging input set that works on ~4.6, then I won't be able to take any responsibility for what might happen to this code in the future from side effects of other changes ;-)

#4 Updated by Erik Lindahl over 5 years ago

Nobody seems interested in helping with it, so I think it's time we kill the feature ;-)

#5 Updated by dawei li about 5 years ago

As mentioned elsewhere, current distance restraints doesn't support DD. However, it works fine if only opemmp (plus GPD) is used to do parallel run, per my test.
It seems not hard to improve the code so that restraints energy is communicated between copies using MPI and each copy only use (openmp+GPU).

I need this feature very much. Otherwise I have to use Charmm,which is too slow.

Is there any possibility to fix this instead of drop it? Thanks.

#6 Updated by Mark Abraham about 5 years ago

dawei li wrote:

As mentioned elsewhere, current distance restraints doesn't support DD. However, it works fine if only opemmp (plus GPD) is used to do parallel run, per my test.
It seems not hard to improve the code so that restraints energy is communicated between copies using MPI and each copy only use (openmp+GPU).

I need this feature very much. Otherwise I have to use Charmm,which is too slow.

Is there any possibility to fix this instead of drop it? Thanks.

I don't understand what you have tested that works and what you think is broken but perhaps fixable. What is "GPD?"

One major barrier to anybody fixing anything is lack of a small-ish test case that someone would like to have working. Providing that would be a useful first step to seeing what is wrong and might be fixable.

#7 Updated by dawei li about 5 years ago

Sorry for the typo. I means GPU.

This one works on both 4.6 and 5.0: simple distance (disre = simple) restraints and use only OPENMP for parallel. Hybrid code using both CPU and GPU is fine too.

For DD, both 4.6 and 5.0 require that two atoms from one restrained pair must be within the same domain. That is, you can't use MPI when doing restrains simulations.

So. the most easy way to include ensemble averaged restraints is to require number of copies equal to number of MPI ranks.

#8 Updated by Mark Abraham about 5 years ago

dawei li wrote:

Sorry for the typo. I means GPU.

This one works on both 4.6 and 5.0: simple distance (disre = simple) restraints and use only OPENMP for parallel. Hybrid code using both CPU and GPU is fine too.

For DD, both 4.6 and 5.0 require that two atoms from one restrained pair must be within the same domain. That is, you can't use MPI when doing restrains simulations.

So. the most easy way to include ensemble averaged restraints is to require number of copies equal to number of MPI ranks.

OK, that confirms my suspicion, so I will make explicit the requirement that ensemble averaging works only with a single domain per simulation.

#9 Updated by Mark Abraham almost 5 years ago

  • Target version changed from 5.x to 5.1

I will do for 5.1 what I suggest in comment 8

#10 Updated by Mark Abraham almost 5 years ago

Mark Abraham wrote:

dawei li wrote:

Sorry for the typo. I means GPU.

This one works on both 4.6 and 5.0: simple distance (disre = simple) restraints and use only OPENMP for parallel. Hybrid code using both CPU and GPU is fine too.

For DD, both 4.6 and 5.0 require that two atoms from one restrained pair must be within the same domain. That is, you can't use MPI when doing restrains simulations.

You can use MPI, you just can't have more than one domain (= MPI rank) per simulation. For a multi-simulation with distance restraints and not replica-exchange, you thus must have as many MPI ranks as simulations, so that each simulation has one rank and thus one domain.

So. the most easy way to include ensemble averaged restraints is to require number of copies equal to number of MPI ranks.

OK, that confirms my suspicion, so I will make explicit the requirement that ensemble averaging works only with a single domain per simulation.

This is already true, I fixed it in 64600020

dawei, what is stopping you from running ensemble-averaged restraints? What error message do you get? You need to compile with MPI, set up a multi-simulation, and run with GMX_DISRE_ENSEMBLE_SIZE environement variable equal to the number of simulations. I can't test that, because nobody has provided a set of inputs that should work. If it's important to be able to run this kind of simulation, please provide a set of inputs (e.g. Verlet-scheme .tpr files) that you think should work. Otherwise, it's not worth my time to do anything other than remove the feature. :-)

#11 Updated by Erik Lindahl over 4 years ago

  • Status changed from New to Blocked, need info
  • Target version changed from 5.1 to future

No feedback in five months, so this will no longer be relevant for 5.1, and unless there is feedback I'd suggest we axe the feature for 6.0.

#12 Updated by Mark Abraham over 3 years ago

We will pull this feature entirely

#13 Updated by Mark Abraham over 3 years ago

It might re-emerge as useful implementation of the external potential module, but nobody's going to fix it in its current form.

#14 Updated by Mark Abraham over 3 years ago

  • Related to Task #1971: Removing buggy features vs. keeping workflows added

#15 Updated by Mark Abraham over 3 years ago

Another user ran into problems at https://mailman-1.sys.kth.se/pipermail/gromacs.org_gmx-users/2016-June/106349.html, because the helpful check acted in a case that merely uses simple restraints in REMD.

#16 Updated by Mark Abraham over 3 years ago

  • Related to Bug #1989: simple distance restraints should work with REMD and multiple ranks per simulation added

#17 Updated by Mark Abraham over 3 years ago

Ensemble distance restraints actually emits a unilateral fatal error in init_disres, so that hasn't worked since before 2009.

Orientation restraints work only with a single domain, and turn on ensemble restraints automatically with multi-simulation. The latter is hinted at in the manual, but the former isn't mentioned anywhere. This is not very usable either.

#18 Updated by Mark Abraham over 3 years ago

  • Related to Bug #408: unwanted ensemble averaging of distance restraints with multisim / replica exchange added

#19 Updated by Gerrit Code Review Bot about 3 years ago

Gerrit received a related patchset '1' for Issue #1117.
Uploader: Berk Hess ()
Change-Id: Ic51230aa19a4640caca29a7d7ff471e30a3d9f09
Gerrit URL: https://gerrit.gromacs.org/6108

#20 Updated by Szilárd Páll about 3 years ago

  • Related to Bug #2029: mdrun gives different distance restraint potential energies depending on the number of openMP threads added

#21 Updated by Berk Hess about 3 years ago

  • Status changed from Blocked, need info to Resolved

#22 Updated by Erik Lindahl almost 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF