ur-whitelab / hoomd-tf

A plugin that allows the use of Tensorflow in Hoomd-Blue for GPU-accelerated ML+MD
https://hoomd-tf.readthedocs.io
MIT License
30 stars 8 forks source link

mapping_fxn has no access to HOOMD box #336

Closed RainierBarrett closed 2 years ago

RainierBarrett commented 2 years ago

In tensorflowcompute.py, the mapping_fxn argument is presently unable to access information about the simulation box, only particle positions. In order to properly treat periodic boundary conditions, the box dimensions are needed.

RainierBarrett commented 2 years ago

This could be addressed in a couple of ways. Easiest is probably to change the documentation and call to mapping_fxn to expect it to take in the simulation box as well as positions.

whitead commented 2 years ago

Is the box not present in a snapshot?

RainierBarrett commented 2 years ago

Sure it is, but I'm talking about the mapping_fxn call expected signature, not the signature of enable_mapped_nlist. Inside of enable_mapped_nlist we say the expected signature of the mapping function is only

:param mapping_fxn: a function whose signature is ``f(positions)`` where positions is an
                            ``Nx4`` array of fine-grained positions and
                            whose return value is an ``Mx4`` array
                            of coarse-grained positions.
:type mapping_fxn: python callable

so when we call it within enable_mapped_nlist we don't presently feed in box information:

g_pos = mapping_fxn(
            snap.particles.position.astype(self.model.dtype))

I should be able to open a PR with a small change either today or tomorrow on this.

whitead commented 2 years ago

Got it, thanks

whitead commented 2 years ago

Closed by #337