search

ahy3nz / coMMParE

Comparing Molecular Mechanics Potential Energies
6 stars 5 forks source link

Cassandra support #15

Closed rsdefever closed 5 years ago

rsdefever commented 5 years ago

Adding support for Cassandra.

Also added an optional parameter, round_decimal to the spawn_engine_simulations() function. This allows the coordinates to be rounded to some precision (in nm), which seems to resolve a fair bit of the energy discrepancies between different engines. I'm imagining that those apparent discrepancies may have been due to the different precision of various coordinate input files.

ahy3nz commented 5 years ago

Unfortunately, this was a little hard to use for me. My conda environment doesn't have python2 (is there a good way to support python2 in a conda environment around python3.6?) Also, my cassandra binary was cassandra_gfortran.exe. It's good that there are checks for these 2 files/executables in your path, but I'm wondering if we can make this a little bit easier

rsdefever commented 5 years ago

My conda environment does not have python2 either -- it just defaults to the system python2 (/usr/bin/python2 in my case). I don't know if there is a good way to support python2 in a python3 anaconda environment. In the case of the executable, I copied my binary from cassandra_gfortran.exe to cassandra.exe. I figured it was easier to require a consistent name for the exec rather than looking for the different possible exec names. I'm open to other ideas/suggestions though -- we are going to run into the same issues integrating cassandra with mbuild.

ahy3nz commented 5 years ago 
CCCC
               bond     angle  dihedral    nonbond        all
gromacs    0.733235  1.208933  0.025839   9.057264  11.085197
openmm     0.581399  0.921252  0.037881   9.161117  10.701648
hoomd      0.581396  0.921258  0.037881  21.715416  23.255952
cassandra  0.000000  0.909460  0.038270   9.137970  10.085690
====================
.Processing LJ and QQ
Processing 1-4 interactions, adjusting neighborlist exclusions
Processing harmonic bonds
Processing harmonic angles
Processing RB torsions
HOOMD SimulationContext updated from ParmEd Structure
C1=CC=CC=C1
               bond     angle  dihedral    nonbond        all
gromacs    1.218464  0.156680  0.270296  29.853940  31.818821
openmm     0.744259  0.002424  0.282690  30.104639  31.134016
hoomd      0.744263  0.002424  0.282821  87.240099  88.269607
cassandra  0.000000  0.003260  0.287120  30.021320  30.311690

In terms of writing out the small cassandra simulation and parsing the energies, this is working for me locally. Good news is that the energies don't appear too distinct.

I do have some concerns in terms of usability:

I think in a perfect world, there would be better python3 integration/call-ability such that a user doesn't need python2 and doesn't need to change their path for library_setup.py. For starters, refactoring into python3 might not be so hard (it looks like refactoring print statements and xrange). From there, maybe this library_setup.py can be an import-able module. If this can get done, library_setup.py won't be an "installation obstacle" for commpare users

For detecting cassandra.exe, could you also include some logic to detect cassandra_gfortran.exe and the other flavors of the cassandra executable? Regardless of compile settings and name of the binary, the calls to cassandra are probably all the same?

I see you tried to keep the cassandra input file consistent with other simulation input files, thanks for that

rsdefever commented 5 years ago

I understand the concerns and don't disagree with them.

Detecting different flavors of the Cassandra executable is easy enough to implement. I can also check for a few 'common' exec names for python2 instead of just the one. Hopefully that alleviates some issues.

Refactoring library_setup.py into python3 isn't a bad idea, but that would have to come with the next Cassandra release. It could be made into an importable module, although I don't immediately see any benefit in the normal usage of Cassandra. In the longer term, it might be possible to remove the library setup step entirely.

I'll try to make the simpler of the above changes later this week. If you prefer to keep Cassandra separate for now that is fine -- I can still use it on my fork.

ahy3nz commented 5 years ago

I think just fixing up how cassandra.exe and library_setup.py are found would be sufficient.

rsdefever commented 5 years ago

I am working on refactoring (and overhauling) library_setup.py to python3. However, given that the current release uses python2, I've made the changes. See if those are what you had in mind.

ahy3nz commented 5 years ago

https://github.com/mosdef-hub/mbuild/pull/636

This mbuild PR will be needed to run this, but for now I'll merge