On Wed, Feb 12, 2014 at 12:16 PM, Ross Walker <ross.rosswalker.co.uk> wrote:
> 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.
>
I wasn't saying do away with code duplication. I was simply saying that at
some point it's easier to have the same code in 5 different subroutines
than to auto-generate those 5 subroutines from the same header by playing
with preprocessor defines. In any case, Scott's way in CUDA is cleaner
(i.e., leave the subroutine definitions out and just have the code in the
header -- that would probably help reduce the #ifdef clutter).
> 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.
>
Well I'll start by saying it only makes sense in explicit solvent (it
fixes divalent metal ions interacting with water), so GB is unnecessary to
support. That said, enforcing all-or-nothing implementations universally
is silly -- a compromise should be struck here. Do as complete a job as
possible given time and expertise constraints and document what works and
what doesn't (and what you think might but aren't sure). Nobody is an
expert in everything and such policies will hinder progress in our science
(or deter developers toward packages like OpenMM ;).
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.
>
This is not at all the model we have currently. There is a lot of
functionality that sander has that pmemd does not. It would be less usual
to support something in pmemd that is unsupported in sander.
Also, the TI code in pmemd is so _drastically_ different from that in
sander (and harder to make new potential terms consistent with it), that
it makes making a new model compatible with TI a much bigger undertaking.
This is done to speed up TI in pmemd (a lot in explicit solvent), but
comes at a cost (it won't work with as much stuff as it will in sander).
Incremental progress here is key to steady improvement.
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.
>
I disagree here. [1] If you're not sure whether or not it works, disable it
and let an expert in the method that wants to use your modification
determine if it works or not. The fastest way to address this complaint
is to have the developer say "it doesn't work" if they're unsure. Why not
give an interested party a head start by saying "it might work, but i'm not
sure"?
This is done already, I believe, with TI + REMD in pmemd which is disabled
but probably works fine. ;)
All the best,
Jason
P.S. TI (only) in pmemd is broken and failing most of the tests. I'm
trying to figure out why.
--
Jason M. Swails
BioMaPS,
Rutgers University
Postdoctoral Researcher
_______________________________________________
AMBER-Developers mailing list
AMBER-Developers.ambermd.org
http://lists.ambermd.org/mailman/listinfo/amber-developers
Received on Wed Feb 12 2014 - 10:00:05 PST
