a proposal for dihedral_fourier.cpp

I don't have access to Loukas Peristeras's email address. Steve can
you forward this question to him?

Problem:
The current version of dihedral_fourier.cpp, each cosine term can be
shifted, but it must be shifted by an integer number of degrees. This
shift is the "di" parameter mentioned in the documentation for that
dihedral_style at:

http://lammps.sandia.gov/doc/dihedral_fourier.html

A shift of 62.5 degrees is not allowed.

I can't think of a reason for this. In the code, Loukas said he did
this for "backwards compatibility". But I I don't understand why we
need backwards compatibility for a new dihedral style. (Was this part
of the code accidentally left over from dihedral_charmm.cpp? If so,
then great, we can fix it easily. See below.)

    I am grateful, excited. But I am also disappointed. This is a
significant problem, especially for large Fourier series. Perhaps
this limitation was not so serious for dihedral_style charmm. However
a shift of half-a-degree is serious for longer fourier series. And
people who want long Fourier series are the target audience.

We actually use Fourier-series dihedral potentials, where a shift of
half a degree leads to qualitatively differences in the behavior of
our coarse-grained protein model. (Bellesia, G.; Jewett, A. I.; Shea,
J.-E. Protein Sci. 2010, 19, 141-154)

  Can we please remove this restriction for this new pair style? I've
attached a slightly modified version of "dihedral_fourier.cpp" which
allows for non-integer angle shifts (based on the Oct28_2012 version
of LAMMPS). I only made modifications on lines 316 and 337 of that
file.

Dear Loukas

   Please let us know if you are open to the idea of non-integer
shifts. Our group uses especially non-standard complicated
potentials. We would be grateful if you are willing to change this.

   I don't mean to sound so crestfallen and unappreciative. I think
it's wonderful that you wrote this new dihedral style, and I intend to
use it (and get other people using it).

Thanks!

Andrew

dihedral_fourier.cpp (11.1 KB)

I'm embarrassed to admit that the file I posted in my previous email
has a bug which I introduced. (My modified version did not read or
write restart files correctly).

I believe I have fixed that issue and am attaching new code to this
post now. (This new version requires changes to both
"dihedral_fourier.cpp" and "dihedral_fourier.h") However I will be
honest that I have not actually tested my latest modified version of
Loukas' code with restart files yet. I will, and when I do, I'll post
again.

Cheers
Andrew

...here are those corrected files

dihedral_fourier.cpp (10.9 KB)

dihedral_fourier.h (1.2 KB)

1) Incidentally, the example provided on the doc page does not work:

http://lammps.sandia.gov/doc/dihedral_fourier.html

(A minor mistake. The dihedral type was not specified in that example.)

Please replace it with this:

dihedral_coeff 1 3 -0.846200 1 0 7.578800 1 0 0.138000 2 -180

2) I also found a very minor bug on line 318 of dihedral_fourier.cpp
(This bug was not introduced by me. It was present in the original version.)

The latest version of dihedral_fourier.cpp is attached.

Sorry for so many posts on this topic.
Cheers!

Andrew

dihedral_fourier.cpp (11 KB)

I tested the most recent version of "dihedral_style fourier" (the
version I modified that supports non-integer shifts), and it
successfully reads and writes restart files. (For convenience, I
attached the most recent version of these files to this message, but
they are the same as the ones I posted earlier.) This code also works
with MPI. (I was afraid my changes would have broken MPI, but it seems
to be okay.)

As expected, this code also works for odd (not even) functions, and it
obeys the dihedral angle sign convention.

Thanks again Loukas. I really like this dihedral_style!

Andrew

dihedral_fourier.cpp (11 KB)

dihedral_fourier.h (1.2 KB)

just one observation.

this whole episode underlines, how much LAMMPSneeds a more systematic unit testing facility and a
better approach to reviewing changes than
“send it to steve and let him figure it out”.

given the massive size of the LAMMPS code base,
this is would be a massive undertaking that would
not be doable by a single person or a small group
of idealists, but require the participation of the entire
LAMMPS community via some kind of “crowdsourcing”.

i would be happy to discuss any suggestions to that
effect. here on the mailing list or in private (preferably
on the list, of course). i could imagine that with a
good idea, there would also be a chance to obtain
some funding to set up the required infrastructure
and dedicate some (hu)man hours to it.

just one observation.

this whole episode underlines, how much LAMMPS
needs a more systematic unit testing facility and a
better approach to reviewing changes than
"send it to steve and let him figure it out".

Reducing the workload on one person seems like a good idea.

Although I have been posting a lot of bug reports lately, I just
wanted to clarify that I did not detect any serious bugs in
dihedral_fourier.

given the massive size of the LAMMPS code base,
this is would be a massive undertaking that would
not be doable by a single person or a small group
of idealists, but require the participation of the entire
LAMMPS community via some kind of "crowdsourcing".

i would be happy to discuss any suggestions to that
effect. here on the mailing list or in private (preferably
on the list, of course). i could imagine that with a
good idea, there would also be a chance to obtain
some funding to set up the required infrastructure
and dedicate some (hu)man hours to it.

I'm just glad nobody seems to mind me posting and ranting on the
mailing list when something does not work the way I expected.

The feedback loop of bug reports and bug fixes is always going to be
really slow when the original author is typically a former grad
student / postdoc who now works for an investment bank somewhere.
Loukas works for scienomics, but I was (still am) some random guy with
a gmail account when Steve accepted my dihedral code last year.

I am discovering that whenever I try to use a LAMMPS feature
contributed by a LAMMPS user, there is a 50% change I will have to
read the code and possibly break out the debugger. As long as people
clearly are prepared for this, it's okay. It's nice to have access to
all these new contributed features. But it's the wild west.
Loukas' new code seems comparatively pretty good.

I do worry about the number of styles and fixes (in various stages of
disrepair and overlapping functionality). I don't think there is a
way to fix them all and keep them all up to date. It would be even
harder check that all the fixes are compatible with each other. (The
number of permutations is huge.)

Axel, what did you have in mind? Here is the best I could come up with:

  -- docs: ancient code warning --
At the top of each doc page for each style or fix, perhaps we could
list how recently it was updated (or successfully used). The same
could apply to utility codes (like amber2lmp). If it's old and
obscure, hopefully users will know not to expect much.

  -- online docs: optional example links ---
A related idea: In addition, each doc page could contain a place for
users (or developers) who want to submit complete working example
input/data files which use that style/fix.

  -- online docs: vote for styles or fixes ---
Since it's impossible to maintain all of the styles and fixes, perhaps
users could vote on which styles/fixes they use the most. Of coarse,
Steve and the other developers would of coarse have freedom to
completely ignore these votes. How to do this and avoid duplicate
votes, astroturfing. It seems like a lot of work.

  -- bug tracker? --
A bug tracker like launchpad.net allows users to vote on the most
pressing bugs. Unfortunately these trackers usually generate a large
number of clueless, duplicate bugs:
http://linuxhaters.blogspot.com/2008/08/one-bug-report-to-rule-them-all.html
http://www.linuxjournal.com/content/ubuntu-bug-reporting
One could try to get around some of these problems if you actually
force users to select which which style or fix they think causing the
problem. (This would eliminate a lot of poorly informed, "why doesn't
this work?", type of bug reports.)
I'm not sure if the benefits of such a system would outweigh the
effort building and maintaining it.

Eventually the number of pair_styles, bonded-interaction styles,
fixes, etc will overwhelm Steve (and the other people like Axel who
support LAMMPS in parallel projects).
Until then, party on

Andrew
If you want me to start a new thread on this topic, I'm happy to.

I'm happy for anyone else (plural) to
take this idea and run with it.

Steve