-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added new to_hoomd method #212
Conversation
I looped in @cbkerr on this for some feedback on the signac integration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any problems with the signac aspect, but I didn't run it with signac.
I think the name to_hoomd
makes it less discoverable. Although it uses the integrator names of HOOMD, it would be much more broadly useful.
Does this imply that we will add functions like to_lammps
in the future? I wouldn't support adding extra overhead to support more backends until the need arises.
What if it were called to_JSON
? People using other tools could change the key names as needed and might not think to search for hoomd.
The only thing making it specific to hoomd is that inertia_tensor
is mapped to moment_inertia
. The to_JSON
function should probably keep coxeter's naming, requring hoomd users to change the key.
Extra feature idea: exporting triangulated meshes to rendering tools like fresnel. Not sure what ovito would need. Would this be helpful?
eg I have a function like this where I get the surfaces needed for fresnel.
def tris_for_fresnel(vertices):
poly = coxeter.shapes.ConvexPolyhedron(vertices)
t=[i for i in poly._surface_triangulation()]
tris=[]
for i in t:
tris.append(i[0])
tris.append(i[1])
tris.append(i[2])
return tris
My current plan is to have a default to_JSON method that exports any subset of keys to a python dict. The to_hoomd method will call to_JSON and a mapping util that renames the keys to match HOOMD's style. This significantly cuts down on code duplication, and allows for future additions of other mapping functions for more complicated file types - for example, OBJ/OFF files to use with ovito and VTK. Let me know any thoughts on this approach! def to_json(self, attributes: list):
"""Get a JSON-serializable subset of shape properties.
Args:
attributes (list):
List of attributes to export. Each element must be a valid attribute
of the class.
Returns
-------
dict
A dict containing the requested attributes.
Raises
------
AttributeError:
If any keys in the input list are invalid.
"""
export = {}
for attribute in attributes:
# If an invalid key is passed, raise default attribute error
export.update({attribute: getattr(self, attribute)})
return export |
Once cbkerr approves these changes I will merge and start working on a new release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Description
This PR adds a new
to_hoomd()
method that allows for easy use of coxeter objects with Signac. The intended use case for this is saving a shape to a job document, which allows for simple access to properties important to the simulation.Motivation and Context
Coxeter works well with simulation tools like
hoomd
, but it would be beneficial to to have a direct export method. While thegsd_shape_spec
is useful for saving simulation data, it does not contain enough data to initialize a simulation in either MC or MD code.Additional tests have been added to the relevant test files:
test_sphere.py::test_to_hoomd
,test_ellipsoid::test_to_hoomd
,test_polygon::test_to_hoomd
,test_polyhedron::test_to_hoomd
, andtest_spheropolyhedron::test_to_hoomd
Types of Changes
1The change breaks (or has the potential to break) existing functionality.
Checklist: