Hi Jason,
I agree that pairs_calc.i is getting out of control. This is part of the
reason I removed the vectorized version of it and have been attempting to
clean things up. The ultimate goal is to simplify this as much as possible
and then go through it with the aim to aggressive optimize for x86_64 - I
think it is crazy trying to keep different code paths for different
architectures these days. That said all of the code duplication is
necessary for performance and to avoid if statements - although I think
with modern compilers and branch prediction it might not be as bad as it
used to be. This is something that needs to be done once everything else
has settled down though so will likely be a goal for between the code
freeze (which is looking and more like a feature freeze given the amount
of changes in the last few weeks) and release.
One thing I would like to avoid though as much as possible is people
adding incomplete new methods to PMEMD. E.g. adding a new VDW potential
but not supporting IPS or GB. We shouldn't be doing this unless there is a
real technical reason for not being able to do it. I.e. if SC-TI would
require major changes to the algorithm to deal with the new VDW term and
the math has not been figured out yet. If the reason for not supporting is
purely one of "I don't use it for GB so I won't add it" then the method
should not be added until everything is supported. Otherwise we will run
the risk of going the way of charmm with tons of different methods that
are only compatible with each other in seemingly random ways.
Since from my understanding 12-6-4 does not involve any algorithmic
clashes with the various methods so I think it should be supported for GB,
IPS, TI and PME and should not be enabled until that is true. This is just
my opinion others may differ on this.
Note, I agree the IPS implementation sucks, it should never have been done
as separate pairs_calc and I am looking to try and simplify this. The main
thing though, if the new method works with TI in sander so it should be
made to work with TI in pmemd - that way we keep the consistency across
sander and pmemd for our users as much as possible.
Also note that this was added to the trunk and yet there are messages
like: "This ...it very well may be supported." - Ideally these things
should have been properly tested and addressed before this was checked in.
Comments welcome.
All the best
Ross
On 2/12/14, 7:49 AM, "Jason Swails" <jason.swails.gmail.com> wrote:
>On Tue, 2014-02-11 at 14:12 -0800, Ross Walker wrote:
>> Awesome - thanks.
>>
>> What about the working with TI and IPS?
>
>I can say with relative confidence that it will not work with IPS. As
>far as TI goes, it should work as-is in sander but will probably not
>work in pmemd (the preprocessor logic in pairs_calc.i is getting _very_
>convoluted and dense -- at some point it's no longer worth that approach
>to avoid duplicating code IMO).
>
>I've made the necessary checks in sander and pmemd for these
>combinations, but it's not guaranteed to work with everything. QM/MM
>stands out, but depending on how the LJ interactions are computed it
>very well may be supported. Time (and bug reports) will tell.
>
>These changes will be checked in as soon as I'm done building and
>testing sander/pmemd.
>
>All the best,
>Jason
>
>--
>Jason M. Swails
>BioMaPS,
>Rutgers University
>Postdoctoral Researcher
>
>
>_______________________________________________
>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 Wed Feb 12 2014 - 09:30:02 PST
