Closed geemi725 closed 3 years ago
Hello @geemi725! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
@whitead instead of implementing this in utils.py shall I only make an example notebook?
@geemi725 think it's better in utils with a conditional import. Also, we have a few trajectories in our examples directory. Can you use one of those for purposes of this? Thanks!
@whitead I updated the example to read from the trajectory and, compute COMs using the HTF function as we discussed. Thoughts?
@geemi725 it looks much better. Can you remove the use of make_tensor_proto
though? I think you want something like
mapped_pos = htf.center_of_mass(tf.cast(positions[:,:3],dtype=tf.float64),
tf.cast(cg_mapping,dtype=tf.float64), box_size)
mdbox = u.dimensions
#Let's generate snapshots of each frame and write to a GSD file
t.append(htf.create_frame(frame_num, CGnum, CGtypes, CGids, mapped_pos.numpy(), mdbox))
frame_num += 1
@whitead I need mapped positions as a numpy array. So to use tf.make_ndarray don't I have to use make_tensor_proto?
I followed this https://www.tensorflow.org/api_docs/python/tf/make_ndarray
I believe just using t.numpy()
will convert a tensor into a numpy array.
@whitead addressed your comment. @hgandhi2411 I think this is good for review now.
@hgandhi2411 addressed your comments. Thanks!!
@geemi725 Everything looks good to me. I fixed some typos. Also, made hoomd context initialization on cpu since initializing on gpu kept giving me a runtime error.
Addresses issue #266