Re: [AMBER-Developers] Code review of pmemd.cuda

From: David Cerutti <dscerutti.gmail.com>
Date: Mon, 17 Apr 2017 18:40:59 -0400

Trailing whitespace is something that I've tried to get rid of, and I don't
think that I've added any more so relative to the original code there
should be a lot less. Something that I also want to tamp down on is
LEADING whitespace versus TABs--my editor likes to auto-indent, and
sometimes I worry this can appear as a TAB in other editors. What I may do
is write a simple python script once I'm all done to eliminate the last
trailing whitespace and make sure that the leading whitespace on every line
is precisely that, so that everyone's editor sees exactly the same thing.


On Mon, Apr 17, 2017 at 6:32 PM, David Cerutti <dscerutti.gmail.com> wrote:

> Confirmed, code review has integrated the TI features from Ross's group.
> I am working through the new kNLCPNE_AFE.h library, nearly done with that
> as well. There is a good deal of code replication across libraries--not
> just in the non-bonded routines of kNLCPNE but also in the particle--mesh
> routines of kPSSE and related libraries. We might look into removing that
> but at present the level is manageable.
>
> Future plans:
> - Implement and test a lookup table for erfc(r)/r based on the base 2
> logarithm of r2... it would work very much like pmemd CPU at that point,
> but with a slightly different indexing system. One of those things that
> just has to be tried to see whether it makes things faster but if lookups
> from texture memory aren't terribly expensive it maintain the same accuracy
> and save exp() and exp2f() in the inner loop.
> - Implement an alternate pipeline for doing the dynamics calculation based
> on the gemstones domain decomposition. A sorting method that keeps atoms
> in both linear and local neighborhood formats, updated at every step, will
> be central to this approach. Developers will then be able to choose
> whether to work with the linear list of atoms or the domain decomposition
> to implement new methods. The code will include numerous stencils for
> importing groups of atoms into __shared__ memory which developers can use
> to implement new energy functions or methods. Once things are in
> __shared__ it's much easier for the uninitiated to write efficeint GPU
> code, so the trick is to make an apparatus for getting there.
>
> Cheers,
>
> Dave
>
>
> On Mon, Apr 17, 2017 at 6:18 PM, Charles Lin <clin92.ucsd.edu> wrote:
>
>> afaik the code review includes the ti code.
>>
>> -Charlie
>>
>> ________________________________________
>> From: Ross Walker [rosscwalker.gmail.com] on behalf of Ross Walker [
>> ross.rosswalker.co.uk]
>> Sent: Monday, April 17, 2017 3:17 PM
>> To: AMBER Developers Mailing List
>> Cc: Daniel Mermelstein; Charles Lin
>> Subject: Re: [AMBER-Developers] Code review of pmemd.cuda
>>
>> Hi Dave,
>>
>> Thanks for doing the code documentation. This will prove very helpful
>> down the line. Modifying the whitespace though will potentially making
>> patching a pain especially the TI code update. The beta is already built
>> against AMBER 16 and, fortunately we have merged everything into master
>> already. Additional betas will be a pain in the butt though since we'll
>> have to maintain 2 versions of the code, one with the new whitespace and
>> one that matches AMBER 16. We may end up having to provide tar files of the
>> entire CUDA directory instead of a patch for future betas which is not
>> ideal (it precludes us making the beta public) but that's something we'll
>> have to figure out.
>>
>> Dan, Charlie - make sure you have all your latest changes merged into the
>> master. Dave please work closely with Dan and Charlie to make sure this
>> goes smoothly.
>>
>> We should also do a sanity check on performance and code correctness
>> although from what you describe this should only be cosmetic changes yes?
>>
>> What do you have planned beyond the code review? We should coordinate
>> things closely on this to make sure it doesn't conflict with what we plan
>> running up to AMBER 18.
>>
>> All the best
>> Ross
>>
>> > On Apr 17, 2017, at 6:00 PM, David Cerutti <dscerutti.gmail.com> wrote:
>> >
>> > To all:
>> >
>> > I am nearly finished with my code review of pmemd.cuda. Comments have
>> been
>> > added throughout with the goal of identifying every function's formal
>> > arguments, clarifying the beginning and end of various long blocks of
>> code,
>> > and better documenting pre-processor directives with a long reach (i.e.
>> > this #endif ends what?).
>> >
>> > While I have tried to preserve Scott's convention of extending
>> assignment
>> > statements to collect related variables into groups, a great deal of
>> white
>> > space has been removed in the interest of adhering to a strict
>> 96-character
>> > limit on each line. This will significantly increase the amount of code
>> > that can fit on the screen at one time, and permit two or more terminal
>> > windows to display separate libraries side by side for easy
>> > cross-referencing. Spacing has also been added or removed to emphasize
>> the
>> > binding order of operations (I have a pretty detailed method, but please
>> > don't feel that it is required for future commits). Additionally, I
>> have
>> > made an effort to decouple braces from pre-processor directives and
>> provide
>> > indentation for the pre-processor directives themselves to help
>> developers
>> > more easily gain a sense of where their code sits in very involved
>> sections
>> > like the non-bonded inner loops in kNLCPNE.h.
>> >
>> > IF YOU HAVE OUTSTANDING EDITS and need to commit them, I have made no
>> > functional changes to the code quite yet--I wanted to get the review
>> done
>> > before I start making other changes. Please send me a mail directly if
>> you
>> > need to merge your items--it is straightforward to go back to the last
>> > commit from two weeks ago and see the code as it was beforehand to make
>> a
>> > diff, but with the review nearly done I will help work out the best way
>> to
>> > do the merge.
>> >
>> > Cheers,
>> > Dave
>> > _______________________________________________
>> > 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 Mon Apr 17 2017 - 16:00:03 PDT
Custom Search