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
pmemd18 patch #486
pmemd18 patch #486
Conversation
The very first attempt to bind PLUMED with high-performance pmemd module of AMBER package. Can only be distributed via diff and not with preplumed/patched files set because of limitations of AMBER license (see http://ambermd.org/LicenseAmber18.pdf section "MODIFICATIONS AND DERIVATIVE WORKS" for details). It's based on old sander14 patch and ported, improved and tested with all available modules of licensed vanilla pmemd18 (including MPI, CUDA and CUDA + MPI) by Drobot Viktor (drobot@belozersky.msu.ru) and Evgeny Kirilin (kirilin@belozersky.msu.ru) NB: on some systems (especially supercomputer clusters) one need to build PLUMED without OpenMP support to avoid spurious crashes with CUDA version of PMEMD.
Hi! I will check it better later. Meanwhile you might want to have a look at the gromacs patch to see how to know from the MD code if PLUMED needs the energy or not. I will try to propose some change to the code in a few days (though I don’t have access to the source code so I will have to ask you to test the code). Thanks a lot! |
Another comment: also download_frc is only needed if you are using energy. In case not, it would be sufficient to upload plumed forces and add them (ideally on the gpu) to amber ones, if there is a call to do it |
The sequence of commands is: prepareCalc After having obtained the frc array: performCalc |
Hello! As from that moment I think that it's all up to you to fix minor
errors and to make further improvements :)
Of course, I will help you with testing that patch.
пн, 10 июн. 2019 г., 22:39 Giovanni <notifications@github.com>:
… The sequence of commands is:
These two first:
prepareCalc
isEnergyNeeded
After having obtained the frc array:
performCalc
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#486?email_source=notifications&email_token=AHUWGJJ5DCLZ5UMSS5PYI5LPZ2UY5A5CNFSM4HWJN5DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXLAGUQ#issuecomment-500564818>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHUWGJLXWOYRHDIXKXUI5H3PZ2UY5ANCNFSM4HWJN5DA>
.
|
AMBER code currently provides routine for adding external forces (the
authors use it for NFE method routines)
If I understand things right way currently PLUMED returns the sum of AMBER
and bias forces so we had to first download and then upload forces; in
other case PLUMED will not know about current forces and the result is
incorrect.
NFE routines return only additional forces so it is possible to use AMBER
gpu_update_frc routine efficiently
пн, 10 июн. 2019 г., 22:34 Giovanni <notifications@github.com>:
… Another comment: also download_frc is only needed if you are using energy.
In case not, it would be sufficient to upload plumed forces and add them
(ideally on the gpu) to amber ones, if there is a call to do it
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#486?email_source=notifications&email_token=AHUWGJLHK642YDO2ELBQ5LLPZ2UDLA5CNFSM4HWJN5DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXK7XWY#issuecomment-500562907>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHUWGJJ2ZVNLDHA53QZJ5G3PZ2UDLANCNFSM4HWJN5DA>
.
|
Dear @dvdesolve sorry for the delay. I suggest a few changes below. Avoid computing energy when it is not neededReplace these lines:
with
(you should declare Also replace this line:
with this line
Please check if results are unchanged both when using Avoid downloading forces when not neededIf the point above works correctly you can eliminate the call to Updating Plumed.cCan you try also updating the included
Thanks! Giovanni |
Hello, dear Giovanni!
Right now I'm participating in Biocatalysis 2019 conference and can't test
proposed changes. But after approx 10 days I will be able to see what can
be done. Thank you for suggestions!
пн, 24 июн. 2019 г., 9:14 Giovanni <notifications@github.com>:
… Dear @dvdesolve <https://github.com/dvdesolve> sorry for the delay. I
suggest a few changes below.
Avoid computing energy when it is not needed
Replace these lines:
if (plumed == 1) then
need_pot_enes = .true.
end if
with
if (plumed == 1) then
call plumed_f_gcmd("prepareCalc"//char(0),0)
plumed_need_pot_enes=0
call plumed_f_gcmd("isEnergyNeeded"//char(0),plumed_need_pot_enes)
if (plumed_need_pot_enes > 0 ) then
need_pot_enes = .true.
end if
end if
(you should declare integer :: plumed_need_pot_enes at the beginning of
the routine)
Also replace this line:
call plumed_f_gcmd("calc"//char(0), 0)
with this line
call plumed_f_gcmd("performCalc"//char(0), 0)
Please check if results are unchanged both when using ENERGY as a CV and
when not using it.
Avoid downloading forces when not needed
If the point above works correctly you can eliminate the call to
gpu_download_frc when plumed_need_pot_enes==0. In particular, notice that
when plumed_need_pot_enes==0 plumed will only *add* forces to the frc
array. This means that you can initialize it to zero, pass it to plumed,
and then add these forces to those computed by pmemd. It is a bit difficult
to propose explicitly a change for this since I do not have access to the
code.
Updating Plumed.c
Can you try also updating the included Plumed.c and Plumed.h files using
those provided with plumed 2.5 (in directory src/wrapper)? The
replacement should be completely transparent (just use the new files
instead of the old ones). The advantage is that:
- the new files provide more options for debugging
- the new files will also work if pmemd is not linked with special
flag --export-dynamic, which should make the executable slightly more
portable.
Thanks!
Giovanni
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#486?email_source=notifications&email_token=AHUWGJJJXVOGOTFKROIDFVLP4BQ2VA5CNFSM4HWJN5DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYL346A#issuecomment-504872568>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHUWGJM35I3RZZJ3XIVXKLTP4BQ2VANCNFSM4HWJN5DA>
.
|
Please check if results are unchanged both when using ENERGY as a CV and when not using it.
Could you also clarify this a bit more? Do you mean test case mentioned in
PLUMED manual?
пн, 24 июн. 2019 г., 10:03 viktor drobot <linux776@gmail.com>:
… Hello, dear Giovanni!
Right now I'm participating in Biocatalysis 2019 conference and can't test
proposed changes. But after approx 10 days I will be able to see what can
be done. Thank you for suggestions!
пн, 24 июн. 2019 г., 9:14 Giovanni ***@***.***>:
> Dear @dvdesolve <https://github.com/dvdesolve> sorry for the delay. I
> suggest a few changes below.
> Avoid computing energy when it is not needed
>
> Replace these lines:
>
> if (plumed == 1) then
> need_pot_enes = .true.
> end if
>
> with
>
> if (plumed == 1) then
> call plumed_f_gcmd("prepareCalc"//char(0),0)
> plumed_need_pot_enes=0
> call plumed_f_gcmd("isEnergyNeeded"//char(0),plumed_need_pot_enes)
> if (plumed_need_pot_enes > 0 ) then
> need_pot_enes = .true.
> end if
> end if
>
> (you should declare integer :: plumed_need_pot_enes at the beginning of
> the routine)
>
> Also replace this line:
>
> call plumed_f_gcmd("calc"//char(0), 0)
>
> with this line
>
> call plumed_f_gcmd("performCalc"//char(0), 0)
>
> Please check if results are unchanged both when using ENERGY as a CV and
> when not using it.
> Avoid downloading forces when not needed
>
> If the point above works correctly you can eliminate the call to
> gpu_download_frc when plumed_need_pot_enes==0. In particular, notice
> that when plumed_need_pot_enes==0 plumed will only *add* forces to the
> frc array. This means that you can initialize it to zero, pass it to
> plumed, and then add these forces to those computed by pmemd. It is a bit
> difficult to propose explicitly a change for this since I do not have
> access to the code.
> Updating Plumed.c
>
> Can you try also updating the included Plumed.c and Plumed.h files using
> those provided with plumed 2.5 (in directory src/wrapper)? The
> replacement should be completely transparent (just use the new files
> instead of the old ones). The advantage is that:
>
> - the new files provide more options for debugging
> - the new files will also work if pmemd is not linked with special
> flag --export-dynamic, which should make the executable slightly more
> portable.
>
> Thanks!
>
> Giovanni
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#486?email_source=notifications&email_token=AHUWGJJJXVOGOTFKROIDFVLP4BQ2VA5CNFSM4HWJN5DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYL346A#issuecomment-504872568>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AHUWGJM35I3RZZJ3XIVXKLTP4BQ2VANCNFSM4HWJN5DA>
> .
>
|
I think it is sufficient to check that an input file without ENERGY and an
input file with ENERGY (and a bias on it) behave in the same way as they do
now.
The goal of this optimization is just to switch off energy calculation when
it is not necessary (that is: when ENERGY variable is not used in
plumed.dat)
Giovanni
…On Mon, Jun 24, 2019 at 12:49 PM drobot viktor ***@***.***> wrote:
> Please check if results are unchanged both when using ENERGY as a CV and
when not using it.
Could you also clarify this a bit more? Do you mean test case mentioned in
PLUMED manual?
пн, 24 июн. 2019 г., 10:03 viktor drobot ***@***.***>:
> Hello, dear Giovanni!
> Right now I'm participating in Biocatalysis 2019 conference and can't
test
> proposed changes. But after approx 10 days I will be able to see what can
> be done. Thank you for suggestions!
>
> пн, 24 июн. 2019 г., 9:14 Giovanni ***@***.***>:
>
>> Dear @dvdesolve <https://github.com/dvdesolve> sorry for the delay. I
>> suggest a few changes below.
>> Avoid computing energy when it is not needed
>>
>> Replace these lines:
>>
>> if (plumed == 1) then
>> need_pot_enes = .true.
>> end if
>>
>> with
>>
>> if (plumed == 1) then
>> call plumed_f_gcmd("prepareCalc"//char(0),0)
>> plumed_need_pot_enes=0
>> call plumed_f_gcmd("isEnergyNeeded"//char(0),plumed_need_pot_enes)
>> if (plumed_need_pot_enes > 0 ) then
>> need_pot_enes = .true.
>> end if
>> end if
>>
>> (you should declare integer :: plumed_need_pot_enes at the beginning of
>> the routine)
>>
>> Also replace this line:
>>
>> call plumed_f_gcmd("calc"//char(0), 0)
>>
>> with this line
>>
>> call plumed_f_gcmd("performCalc"//char(0), 0)
>>
>> Please check if results are unchanged both when using ENERGY as a CV and
>> when not using it.
>> Avoid downloading forces when not needed
>>
>> If the point above works correctly you can eliminate the call to
>> gpu_download_frc when plumed_need_pot_enes==0. In particular, notice
>> that when plumed_need_pot_enes==0 plumed will only *add* forces to the
>> frc array. This means that you can initialize it to zero, pass it to
>> plumed, and then add these forces to those computed by pmemd. It is a
bit
>> difficult to propose explicitly a change for this since I do not have
>> access to the code.
>> Updating Plumed.c
>>
>> Can you try also updating the included Plumed.c and Plumed.h files using
>> those provided with plumed 2.5 (in directory src/wrapper)? The
>> replacement should be completely transparent (just use the new files
>> instead of the old ones). The advantage is that:
>>
>> - the new files provide more options for debugging
>> - the new files will also work if pmemd is not linked with special
>> flag --export-dynamic, which should make the executable slightly more
>> portable.
>>
>> Thanks!
>>
>> Giovanni
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub
>> <
#486?email_source=notifications&email_token=AHUWGJJJXVOGOTFKROIDFVLP4BQ2VA5CNFSM4HWJN5DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYL346A#issuecomment-504872568
>,
>> or mute the thread
>> <
https://github.com/notifications/unsubscribe-auth/AHUWGJM35I3RZZJ3XIVXKLTP4BQ2VANCNFSM4HWJN5DA
>
>> .
>>
>
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#486?email_source=notifications&email_token=ABBNCXRNUEOFL36EYFAXCRTP4CRCPA5CNFSM4HWJN5DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYMRA4A#issuecomment-504959088>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABBNCXSNI7YFCZNPP2QNHA3P4CRCPANCNFSM4HWJN5DA>
.
|
Hello! Now I'm able to test proposed changes.
I've changed source code as you suggested. It compiles just fine but when I try to run it crash is observed:
Seems like exception is thrown during |
@dvdesolve sorry for the delay. I think you are right. It is a bit difficult to fix stuff without having access to the code. Would it be possible to move all the calls to Giovanni |
Hello! Sorry for my late response. |
In any case because we only need to know about the necessity of computation of potential energies we may ignore actual content of variables needed for request |
ping |
I am not sure this will work. There are some internal checks to avoid that one use I am not sure what we should do at this point. Perhaps a useful information would be to know how much is the slowdown. I mean: if you change the patch assuming that the user does not need the total energy, how much is the speed increase? Thanks! |
…th latest AMBER updates
Codecov Report
@@ Coverage Diff @@
## master #486 +/- ##
==========================================
+ Coverage 83.84% 83.86% +0.02%
==========================================
Files 579 579
Lines 42927 43019 +92
==========================================
+ Hits 35993 36079 +86
- Misses 6934 6940 +6
Continue to review full report at Codecov.
|
I've just tested performance on the following system: Arch Linux 64 bit, CUDA 10.1.243, GCC 7.4.1, NVIDIA GeForce GTX 750, Intel Core i7-4790 CPU / 3.60GHz. Simulation protocol: NVT, 10234 atoms, dt = 0.001. With explicit computation of potential energies performance is 37.76 ns/day |
Regarding your suggestion in this comment
Current implementation of CUDA support in PMEMD module offers two directives for dealing with forces upload on GPU card - Also it's still inevitable to perform |
I've replaced old wrapper to a new one. However, changing With out_calc.log Seems like the first call to |
Regarding soon deadline for 2.6 freeze it would be nice to finish testing and optimization of this patch to include it into 2.6.0 release:) |
@dvdesolve in the worst case we include your current patch and add the fixes for ENERGY later. Sorry for lagging behind in this. To summarize the state of the patch, please correct me if I am wrong:
Point 2 (finding if energy is needed)About the second point, I see that at the time of the first call (to "prepareCalc") you do not have access to positions. The fix would be thus the one that is used for the NAMD patch. In particular you should do:
in the first call and then
in the second call. The following "equalities" hold: prepareDependencies should be able to find out if energy is needed but will not access to positions yet, so should also work with the code as it is arranged now. The names are not particularly good, and are mostly a frozen accident related to how we stepwise decomposed the steps to allow better optimizations of the interfaces. Points 3 (using gpu_upload_frc_add)In case ENERGY is not used (and only in that case) PLUMED will just add numbers to the force array. (For comparison: when ENERGY is used, PLUMED will also scale the numbers in the energy array). This means that you could do as follows:
Thanks again for taking care of this, it is a very useful and appreciated effort! Giovanni |
I'll try to implement points 2 and 3 and tell you the results. Thank you. |
I have implemented request for energy as you suggested, seems like all looks great. You can check the results for simple forces-on-energy calculation made with However I have a question. What do you mean under second call? Currently we've the following layout of calls to the PLUMED:
Also we have
Do you want to replace line with
Only with these modifications we're getting inconsistencies in writing |
I see now, the problem is with the two calls, one outside the loop and one
in the loop! The way I suggested is likely incorrect since you are only
checking at the first step. I mean: if someone uses multiple time stepping
on ENERGY to bias it every 2 steps, you will still download it at every
step.
Every time you have a cmd("calc") you should split it in the following
consecutive steps:
cmd("setStep")
cmd("prepareDependencies")
cmd("isEnergyNeeded")
now you are in the position of deciding if energy is needed or not. Thus
the first part should go before you have to communicate to amber what to do.
Only after that you can use the next two steps:
cmd("shareData")
cmd("performCalc")
setting of positions/forces/virial etc can be anywhere after "setStep" and
before "shareData".
Giovanni
…On Tue, Sep 10, 2019 at 11:48 AM drobot viktor ***@***.***> wrote:
I have implemented request for energy as you suggested, seems like all
looks great. You can check the results for simple forces-on-energy
calculation made with pmemd.
results.tar.gz
<https://github.com/plumed/plumed2/files/3594911/results.tar.gz>
However I have a question. What do you mean under *second call*?
Currently we've the following layout of calls to the PLUMED:
INIT CALL ! (Plumed_init.inc)
--- step 0 of dynamics ---
need_pot_enes = .true. ! (invoked by MD code)
FORCE CALL ! (Plumed_force.inc)
REQUEST CALL ! (as suggested by you via plumed_need_pot_enes trick)
--- main MD loop starts here ---
FORCE CALL ! (Plumed_force.inc)
--- main MD loop ends here ---
Also we have Plumed_force.inc file which contains:
...
call plumed_f_gcmd("setVirial"//char(0), plumed_virial)
call plumed_f_gcmd("setBox"//char(0), plumed_box)
call plumed_f_gcmd("calc"//char(0), 0);
...
Do you want to replace line with calc call to the following?:
call plumed_f_gcmd("shareData"//char(0), 0)
call plumed_f_gcmd("performCalc"//char(0), 0)
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#486?email_source=notifications&email_token=ABBNCXX2WVQGYSG7GUTEUYTQI5UNPA5CNFSM4HWJN5DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6KQO5Y#issuecomment-529860471>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABBNCXXUOMPTAR5RSXBYONTQI5UNPANCNFSM4HWJN5DA>
.
|
|
Also one thing to note: seems like energy array is only for printout/debugging purposes. By default no energy calculation is performed and you can only get it on-demand with |
At a first approximation, can't we just disable the ENERGY CV with PMEMD18? |
This means that you should move this part of the code
to before that AMBER's routine. Is that possible? |
That's of course an alternative solution! But I think the correct solution is very close... |
This is not a problem. Plumed does not change the energy. If ENERGY is biased, it scales the forces (that's why you have to download them first in that case). |
I asked just because of this sentence:
As for now we're calling
Am I right that it's enough to use the following before any dynamics step?:
and then just continue with the rest of |
Almost! To be precise, you should also change the "calc" call to "shareData" + "performCalc". Since |
And it's done. Here are the results for energy biasing.
That's the price for communication with GPU on every step for MetaD :) If only we could perform all necessary actions on card and communicate with it once per |
No way... variables are needed at every step they are biased, not just when the potential is updated! |
I understand, what I'm talking about requires GPU implementation of PLUMED routines, am I right? |
Yes. Another gain (easier) could come from overlapping Amber GPU and PLUMED
CPU. That’s what we do in gromacs.
Il giorno mar 10 set 2019 alle 18:04 drobot viktor <notifications@github.com>
ha scritto:
If only we could perform all necessary actions on card and communicate
with it once per PACE period to add new Gaussians...
No way... variables are needed at every step they are biased, not just
when the potential is updated!
I understand, what I'm talking about requires GPU implementation of PLUMED
routines, am I right?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#486?email_source=notifications&email_token=ABBNCXVJAHL3XRNTFHM3WJLQI7ARNA5CNFSM4HWJN5DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6LUHNQ#issuecomment-530006966>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABBNCXV6PDZIP3WFYMSKYM3QI7ARNANCNFSM4HWJN5DA>
.
--
Sent from Gmail mobile
|
Main bottleneck is transmission of forces and/or coordinates to/from GPU. PMEMD code organized in that way so all calculations and temporary data are kept on GPU and synced only for printout. However, there are some exceptions - NFE methods are just an example of them and uses the same strategy as we do in current patch. |
Yes. Overlapping calculation does not decrease that, it is only relevant
for expensive CVs.
In case there is a method in Amber to download/upload coordinates/forces of
selected atoms, after “prepareDependencies” it’s possible to retrieve the
list of needed atoms. Transferring less atoms might speed up things.
Another possible trick is multiple time stepping (Ferrarotti et al JCTC
2014). However implementing this in an automatic way would require some
change within Plumed to allow retrieving a flag saying that no action was
activated at that step. Easy to do. If you want to try hard coding the
calls to Plumed only at even steps (step%2==0) and this makes the
calculation significantly faster we can add this feature.
Giovanni
Il giorno mar 10 set 2019 alle 18:12 drobot viktor <notifications@github.com>
ha scritto:
Min bottleneck is transmission forces and/or coordinates to/from GPU.
PMEMD code organized in that way so all calculations and temporary data are
kept on GPU and synced only for printout. However, there are some
exceptions - NFE methods are just an example of them and uses the same
strategy as we do in current patch.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#486?email_source=notifications&email_token=ABBNCXTUU7WHUN56I55UHHDQI7BOJA5CNFSM4HWJN5DKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6LVAOQ#issuecomment-530010170>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABBNCXXT5Z36D4ZDXWW7WPDQI7BOJANCNFSM4HWJN5DA>
.
--
Sent from Gmail mobile
|
As for now it seems that there are no methods for partial update of forces on GPU |
@dvdesolve if you think this is complete I can merge it. I will then contact AMBER developers to see if we can incorporate it directly. Thanks again! |
Yes, I think you can merge it. Thank you for help! |
Writing here just to not raising another issue. Since AMBER20 release both |
@dvdesolve @GiovanniBussi Hey here, I wonder how to run metadynamics simulation using the latest AMBER22 with Plumed plugin? Are there some tutorials showing how to setup the input files? Thanks! |
Hi, did you check masterclass 22.13 by @andrea-arsiccio ? I am pretty sure there is an example there. |
Patch for PMEMD18 module of AMBER package