Re: [AMBER-Developers] Oops, I introduced a bug into pmemd.cuda_SPFP (master branch), but how shall I resolve it?

From: Jason Swails <>
Date: Wed, 25 Oct 2017 16:52:43 -0400


> On Oct 25, 2017, at 4:04 PM, Daniel Roe <> wrote:
>> On Wed, Oct 25, 2017 at 1:09 PM, David Cerutti <> wrote:
>> Beware, I've just realized that pmemd.cuda_SPFP in the master branch has a
>> bug that shows up in the constant pH routines (those results are bad).
>> DPFP is fine. I've determined what I think the problem--something I did
>> about two months ago with the intention of clarifying things, but I didn't
>> go far enough. It only shows up in the constant pH routines because things
>> are getting refreshed frequently, mixing situations where the intended
>> change takes effect and those where it does not.
> Yikes - glad you caught this! I can confirm that this is an issue; the
> CUDA explicit solvent constant pH test case energies are off by
> several thousand kcal/mol. Are you sure constant pH is the only thing
> affected? One of the non-eq. free energy test cases
> (test/cuda/nfe/pmd) also has an energy that is off by about 1 kcal/mol
> which seems like too much.
> Until we have working CI testing in Amber it is incumbent upon all
> developers to test test **test** the code before committing to master.
> I know that several developers (myself included) use the GIT master
> branch for production dynamics. I am in fact running some constant pH
> simulations now but fortunately I'm using the Amber 16 version (which
> I've just checked does pass the test case) so I don't need to throw
> out any results.
> Developers who are working with pmemd.cuda should always run the SPFP
> tests in addition to the default DPFP, especially since this is the
> code that users will actually be using 99% of the time. Maybe we
> should have different test outputs for every shader model type as
> well?
> Again, I think this confirms that we absolutely need CI testing. There
> is no way that a bug like this (that is detected by an existing test
> case) should have ever made it into the master branch. I can look into
> the possibility of hosting a server at NIH that will do this, although
> it may be tricky since there are a lot of hoops to jump through for
> setting up a public-facing web host.

My jenkins server is already running amber jobs. We can just piggy-back off that server.

The main problem is that we also need something like gitlab that can implement merge gates to enforce the policy.

In the meantime it’d be pretty easy to set up a reactive system that emails people after it breaks.

All the best,

> -Dan
>> The change was this: there is an array of float2 (two fp32s) called
>> pbAtomChargeSPLJID. That is, "atom charge (single precision) PLUS Lennard
>> Jones type index." It's good to couple these things for memory access
>> efficiency, but it's bad because at some point we end up representing an
>> integer as a float, then casting it back to an integer, to index into
>> another array (Lennard Jones parameter tables). This is quite safe for the
>> Lennard Jones typing--integers in the range +/- 2^23 can be represented
>> exactly by fp32. But it's the whole issue of INTENT in the way that array
>> is handled--I made it so that the charge was loaded as a float (no change),
>> but the LJID was loaded as a __uint_as_float(), then read back as a
>> __float_as_uint() to explicitly preserve the bitwise representation. (This
>> can also be done with unions.) Speed is not an issue for this case--the
>> conversion back to integer happens outside a loop where the real work gets
>> done.
>> But, before I fix this, I'd like to float (!) the question of what to do in
>> these situations. It looks clunky to explicitly load and unload an
>> unsigned int into the float2 data--but it serves as a reminder of what is
>> really happening and in some situations it will be faster.
>> Dave
>> _______________________________________________
>> AMBER-Developers mailing list
> --
> -------------------------
> Daniel R. Roe
> Laboratory of Computational Biology
> National Institutes of Health, NHLBI
> 5635 Fishers Ln, Rm T900
> Rockville MD, 20852
> _______________________________________________
> AMBER-Developers mailing list
AMBER-Developers mailing list
Received on Wed Oct 25 2017 - 14:00:03 PDT
Custom Search