Add hoomd2 support, with tutorials #27

DavyYue · 2017-06-13T17:50:52Z

Just want to get this started. I'm sure there is some code cleanup that needs to be done.

This involves writing new hoomd headers that get written when saving a hoomd runscript. In MSIBI.optimize(), the hoomd version is deduced with a try-except block. In workers.py, the _hoomd_worker() now uses the correct executable depending on the hoomd version, because hoomd1 was a standalone executable, whereas hoomd2 is now a python module.

This makes more sense because MDTraj reads hoomd xml files as so if they have a .hoomdxml extension

ctk3b · 2017-06-13T17:52:35Z

msibi/optimize.py

@@ -62,7 +62,8 @@ def __init__(self, rdf_cutoff, n_rdf_points, pot_cutoff=None, r_switch=None,
        self.dr = rdf_cutoff / (n_rdf_points - 1)
        self.smooth_rdfs = smooth_rdfs
        self.rdf_r_range = np.array([0.0, self.rdf_cutoff + self.dr])
-        self.rdf_n_bins = self.n_rdf_points + 1
+        # got rid of +1 because mdtraj now # does this as we expect
+        self.rdf_n_bins = self.n_rdf_points


👍 but remove the comment before finalizing things

I will be sure to do that next time

ctk3b · 2017-06-13T17:53:38Z

msibi/state.py

                 name=None, backup_trajectory=False):
-        self.kT = k * T
+        self.kT = kT


Is this backwards compatible with HOOMD 1?

I don't think so. HOOMD documentation states for v2.0.0

Integrators now take kT arguments for temperature instead of T to avoid confusion on the units of temperature.

under the heading

Changes that require job script modifications

Yeah, they just changed the syntax to the integrators. I think this is probably cleaner instead of passing k and T separately.

Ok so do we just want to completely drop 1.0 support then? Or are we going to divide the user input by k and then passing that along for HOOMD 1.0 users?

In HOOMD 1.0, the value you pass to the integrator is still kT, but the parameter you pass is called T. They changed it to kT because everybody new to HOOMD seemed to have issues with that, so this is still good for HOOMD. We'll have to make this clear in the docs that for other engines you make have to pass kT, e.g., state = State(kT=305*8.314e-3, ...)

ctk3b · 2017-06-13T17:54:20Z

msibi/state.py

 from hoomd_script import *

-system = init.read_xml(filename="{0}", wrap_coordinates=True)


You sure this needs to be deleted? This was working correctly in HOOMD 1.x, right?

Good catch, I think this is just an accidental deletion

I think the deleted line has been replaced by the following line system = read_xml(filename="{0}", wrap_coordinates=True)

It does indeed need to be init.read_xml(...)

ctk3b · 2017-06-13T17:56:26Z

Looks like it's coming along. Just make sure to tidy up all the stray comments and polish up code to be PEP8 compliant.

ctk3b · 2017-06-13T18:07:15Z

setup.py

@@ -28,13 +28,13 @@ def run_tests(self):
      version='0.1',
      description='',
      url='http://github.com/ctk3b/misibi',
-      author='Christoph Klein, Timothy C. Moore',
-      author_email='[email protected], [email protected]',
+      author='Christoph Klein, Timothy C. Moore', 'Davy Yue'


This field is typically used for the official package maintainers (i.e. people who approve PRs and distribute the package via PyPI/conda). We're more than happy to add you once you get to that point but for now you fall into the "contributor" category :)

One thing you could add is module doc strings to the entire project like this. That would provide an intermediate level of authorship acknowledgement between maintainers and the github contribution list

tcmoore3

Looking good

tcmoore3 · 2017-06-13T19:48:08Z

msibi/state.py

                 name=None, backup_trajectory=False):
-        self.kT = k * T
+        self.kT = kT


Yeah, they just changed the syntax to the integrators. I think this is probably cleaner instead of passing k and T separately.

tcmoore3 · 2017-06-13T19:52:26Z

msibi/tutorials/lj/opt.py

 # Clear out the temp files
+# back up, not hard coded, not file-based (development dependent)
+# later - storing data (backups potentially), some fiels may not exist in future


Clean up comments

Yeah I'll do a scan through the code and delete all the stray comments

tcmoore3 · 2017-06-13T19:53:34Z

msibi/tutorials/lj/state1/hoomd_run_template.py

+# **** 2017_05_31 Notes ******
+# update file to correct hoomd 2.0 syntax
+# look up docs
+# combine with state.py in main folder


Probably not the best idea to take notes within the source code...

tcmoore3 · 2017-06-13T19:53:59Z

msibi/tutorials/lj/state1/hoomd_run_template.py

-run(1e3)
+all = hoomd.group.all()
+nvt_int = hoomd.md.integrate.langevin(group=all, kT=T_final, seed=1) # possibly use kT instead of T???
+hoomd.md.integrate.mode_standard(dt=0.001) #integrate.\*_rigid() no longer exists. Use a standard integrator on group.rigid_center(), and define rigid bodies using constrain.rigid()


Clean up comments

tcmoore3 · 2017-06-13T19:54:08Z

msibi/tutorials/lj/state2/hoomd_run_template.py

+# **** 2017_05_31 Notes ******
+# update file to correct hoomd 2.0 syntax
+# look up docs
+# combine with state.py in main folder


tcmoore3 · 2017-06-13T20:00:15Z

msibi/workers.py

        else:
            logging.info('    Running state {state.name} on CPU'.format(**locals()))
-            cmds = ['hoomd', 'run.py']
+            cmds = [executable, 'run.py']


@ctk3b This was causing unreported failures, which we've pointed out in #8 needs to be caught. Besides that, is this the best way to make sure we launch the right executable? Will mpiexec play nicely with python on a cluster?

If we're only using one rank then this should just be whatever the most straight forward way to launch HOOMD 2.0 is. I'm assuming that would not involve an explicit call to mpiexec?

Indeed if each CG simulation is run on a single GPU then mpiexec is not needed. We will just support single GPU CG simulaitons for now and then worry about multi-GPU runs in the future?

That seems like an easy way to start since it appears that you were only launching one rank per sim anyways, right? This shouldn't have been a major limitation for the system sizes you've been working on.

tcmoore3

Looks good.

ctk3b · 2017-06-14T17:28:08Z

setup.py

-      author_email='[email protected], [email protected]', '[email protected]'
+      author='Christoph Klein, Timothy C. Moore'
+      author_email='[email protected], [email protected]'
+      contributors='Davy Yue'


Have you tested this? I don't think this is a valid keyword and is not what I was suggesting. Sorry if I was unclear. By contributors, I just meant that you are a contributor to the package which is picked up here https://github.com/mosdef-hub/msibi/graphs/contributors

http://setuptools.readthedocs.io/en/latest/setuptools.html?highlight=metadata#metadata

I removed the contributors='Davy Yue'

ctk3b · 2017-06-14T17:34:33Z

setup.py

+# Contributors: Tim Moore, Davy Yue
+#
+##############################################################################
+


For the module level doc strings, I was suggesting mimicking what packages like MDTraj do where every file has a header with the project description, copyright, license and a list of people who wrote THIS particular file and contributed to it - not the package as a whole. So for example, remove all of your changes from setup.py and add this header to all the .py files within msibi and you can add yourself to the ones that you contributed relevant code too. However, please do this in a separate pull request as this will become quite nasty to review.

ctk3b · 2017-06-14T17:40:43Z

msibi/tutorials/lj/state0/hoomd_run_template.py

+# **** 2017_05_31 Notes ******
+# update file to correct hoomd 2.0 syntax
+# look up docs
+# combine with state.py in main folder


Last remaining stray comments

Davy Yue added 4 commits June 13, 2017 12:26

Add Davy Yue to author list in setup.py

6cc6fe9

Change start.xml files to start.hoomdxml in tutorial

00f0d10

This makes more sense because MDTraj reads hoomd xml files as so if they have a .hoomdxml extension

Clean up LJ tutorial

905b015

ctk3b reviewed Jun 13, 2017

View reviewed changes

tcmoore3 reviewed Jun 13, 2017

View reviewed changes

tcmoore3 mentioned this pull request Jun 14, 2017

Update travis, improve pair selection #28

Merged

DavyYue added 8 commits June 14, 2017 11:27

Clean up optimize.py

7a52a85

Cleanup state.py

8eb47ac

Cleanup workers.py

9c47294

Cleanup setup.py

376ec29

Remove unnecessary files from tutorial

a733bed

Cleanup opt.py in lj tutorial

8be834f

Cleanup lj tutorial

633cfa7

Add .gitignore

980b867

tcmoore3 approved these changes Jun 14, 2017

View reviewed changes

ctk3b reviewed Jun 14, 2017

View reviewed changes

ctk3b requested changes Jun 14, 2017

View reviewed changes

DavyYue added 3 commits June 14, 2017 13:45

Remove stray comments

28527af

Cleanup setup.py docstring

46fdac4

Cleanup setup.py commas

cb0840e

tcmoore3 merged commit b68f465 into mosdef-hub:master Jun 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add hoomd2 support, with tutorials #27

Add hoomd2 support, with tutorials #27

DavyYue commented Jun 13, 2017

ctk3b Jun 13, 2017

DavyYue Jun 13, 2017

ctk3b Jun 13, 2017

DavyYue Jun 13, 2017

tcmoore3 Jun 13, 2017

ctk3b Jun 13, 2017

tcmoore3 Jun 14, 2017

ctk3b Jun 13, 2017

tcmoore3 Jun 13, 2017

DavyYue Jun 13, 2017

tcmoore3 Jun 13, 2017

ctk3b commented Jun 13, 2017

ctk3b Jun 13, 2017

tcmoore3 left a comment

tcmoore3 Jun 13, 2017

tcmoore3 Jun 13, 2017

DavyYue Jun 13, 2017

tcmoore3 Jun 13, 2017

tcmoore3 Jun 13, 2017

tcmoore3 Jun 13, 2017

tcmoore3 Jun 13, 2017 •

edited

Loading

ctk3b Jun 13, 2017

tcmoore3 Jun 14, 2017 •

edited

Loading

ctk3b Jun 14, 2017

tcmoore3 Jun 14, 2017

tcmoore3 left a comment

ctk3b Jun 14, 2017

DavyYue Jun 14, 2017

ctk3b Jun 14, 2017

ctk3b Jun 14, 2017

		from hoomd_script import *

		system = init.read_xml(filename="{0}", wrap_coordinates=True)

Add hoomd2 support, with tutorials #27

Add hoomd2 support, with tutorials #27

Conversation

DavyYue commented Jun 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ctk3b commented Jun 13, 2017

Choose a reason for hiding this comment

tcmoore3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tcmoore3 Jun 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tcmoore3 Jun 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tcmoore3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tcmoore3 Jun 13, 2017 •

edited

Loading

tcmoore3 Jun 14, 2017 •

edited

Loading