Re: [AMBER-Developers] gpu_update_box

From: David Cerutti <dscerutti.gmail.com>
Date: Fri, 18 Mar 2022 15:10:17 -0400

What's going on here is that not all unit cells are orthorhombic--such
would imply that the box angles alpha, beta, and gamma are always 90
degrees. The general unit cell is triclinic, where alpha, beta, and gamma
can take on any values (with some degree of excluded territory, as certain
combinations would fail to make a shape that can exist). The code you are
seeing are seeing was created in response to another aspect of the original
pmemd.cuda pair list that, if I recall correctly, was missing interactions
from time to time due to a miscommunication between the way the pair list
was generated versus the way the box had changed. I don't recall if it was
creating energy drift or causing an outright crash. It was also around the
time I added the -AllowSmallBox flag: note that the code will not allow
users to run simulations that are less than three pair list subdivisions
along any dimension without an explicit instruction to do so, as I wasn't
sure that the way the particles get re-imaged at the beginning of each time
was safe if there are only two subdivisions.

But the HessianNorms function is calculating the actual thickness of each
subdomain cell, as defined by the distance separating the planes of
opposing faces. This is integral to the reason that, unless you do some
sophisticated stuff like the honeycomb idea I had, a truncated octahedron
is not as efficient as the 22% reduction in overall particle count would
imply: the total volume of each cell needs to expand as it tilts, and this
implies more tiles to cover the same volume. It's still advantageous to
use a truncated octahedron, as the Hilbert Space curve does pretty well to
localize what is needed and of course the cutoff shell of any given
particle remains spherical.

What's the ETA on the energy drift fix? Is this an atomic change, or does
it also come with other refactoring / rollback of various precision trims?

Dave


On Fri, Mar 18, 2022 at 2:14 PM Scott Le Grand <varelse2005.gmail.com>
wrote:

> Let's look at this, here's the new code:
> HessianNorms(invU, cdepth);
> double skin = cdepth[0]/gpu->sim.xcells - gpu->sim.cut;
> double yskin = cdepth[1]/gpu->sim.ycells - gpu->sim.cut;
> double zskin = cdepth[2]/gpu->sim.zcells - gpu->sim.cut;
>
> The array cdepth is filed with the new a, b, and c by means of an external
> call. But as far as I know, all unit cells are orthorhombic, so we've
> always been able to use:
> // Recalculate a, b, c
> double a =
> pNTPData->ucell[0];
> double b =
> sqrt(pNTPData->ucell[1] * pNTPData->ucell[1] +
>
> pNTPData->ucell[4] * pNTPData->ucell[4]);
> double c =
> sqrt(pNTPData->ucell[2] * pNTPData->ucell[2] +
>
> pNTPData->ucell[5] * pNTPData->ucell[5] +
>
> pNTPData->ucell[8] * pNTPData->ucell[8]);
>
> because of the constraints that imposes on the column space of the unit
> cell matrix.
>
> So either the first code clause is overkill and repeated work or we have to
> revisit the previous orthorhombic assumption. Which is it?
>
> But even then, why would one change the unit cell matrix without updating
> the a, b, and c derived from it? And yes, I'm impatient w/r to fixing bugs.
> Guilty.
>
>
> On Fri, Mar 18, 2022 at 10:42 AM Scott Le Grand <varelse2005.gmail.com>
> wrote:
>
> > If the unit cell changes, wouldn't a, b, and c change as well since they
> > are derived from said unit cell?
> >
> > As for the speed of response. If it's important to anyone, they'll
> > respond. If not, well they can make their principled stand against me and
> > my impatience later. Otherwise, I acknowledge your point but also my
> > okeydokey here, nobody is debugging this thing. It's an easy fix. And I
> > need to do that fix to make it exploit the fixed point local coordinate
> > system, but I could leave the seemingly sloppy incorrect behavior here.
> > Driveby contributions to PMEMD like this however are just not good IMO
> and
> > why I threw my hands up in frustration after trying to refactor for
> > performance in 2020. It's a Sisyphusian effort and the tsunami of
> > unattended warning messages and dead code in the thing alone is enough to
> > make one run screaming into the night IMO. It was a single unitialized
> > variable that broke PMEMD for Silicon Therapeutics in 2020. But also, why
> > is there is so much dead code?
> >
> > The author of this code added a new and completely superfluous method of
> > recalculating a, b,and c without exploiting the properties of the
> > orthorhombic coordinate system to do it more simply as it is done in
> > gpu_pressure_scale, which itself does the same thing internally as
> > gpu_update_box if there's an MC barostat. And I hope you can forgive me
> for
> > being annoyed at the needless repetition of functionality here w/o any
> > notes as to why this was necessary.
> >
> > On Fri, Mar 18, 2022 at 10:22 AM David A Case <david.case.rutgers.edu>
> > wrote:
> >
> >> On Fri, Mar 18, 2022, Scott Le Grand wrote:
> >>
> >> >Okeydokey, nobody cares. Therefore it's a bug and I'm going to fix it.
> >> Test
> >> >will be updated to reflect this. What SM revision should I use for the
> >> test
> >> >reference files?
> >>
> >> ??!? How fast do you expect people to respond? I looked at this and
> >> didn't
> >> see any obvious error -- the two routines are called at different
> places:
> >> gpu_pressure_scale only when ntp>0 and barostat=1, whereas
> gpu_update_box
> >> is only called when re-weighting trajectories.
> >>
> >> But I don't know the code here. Clearly, you must be convinced that
> a,b,c
> >> need to be updated when reweight is > 0. I'll try to look at what
> >> behavior
> >> changes when you make the change. But I'm travelling to the ACS meeting
> >> tomorrow, so you may disappointed if you require immediate feedback.
> >>
> >> ....dac
> >>
> >>
> >> _______________________________________________
> >> AMBER-Developers mailing list
> >> AMBER-Developers.ambermd.org
> >> http://lists.ambermd.org/mailman/listinfo/amber-developers
> >>
> >
> _______________________________________________
> AMBER-Developers mailing list
> AMBER-Developers.ambermd.org
> http://lists.ambermd.org/mailman/listinfo/amber-developers
>
_______________________________________________
AMBER-Developers mailing list
AMBER-Developers.ambermd.org
http://lists.ambermd.org/mailman/listinfo/amber-developers
Received on Fri Mar 18 2022 - 12:30:03 PDT
Custom Search