cmelab / morphct

GNU General Public License v3.0
0 stars 6 forks source link

Fix `_set_center` function in chromophores.py #31

Closed jennyfothergill closed 2 years ago

jennyfothergill commented 2 years ago

Currently we're doing some goofy stuff in a while loop to determine the image and wrapped position of the unwrapped centers:

    def _set_center(self, snap, atom_ids):
        box = snap.configuration.box[:3]
        unwrapped_pos = snap.particles.position + snap.particles.image * box
        center = np.mean(unwrapped_pos[atom_ids], axis=0)
        img = np.zeros(3)
        while (center + img * box < -box / 2).any() or (
            center + img * box > box / 2
        ).any():
            img[np.where(center < -box / 2)] += 1
            img[np.where(center > box / 2)] -= 1
        self.unwrapped_center = center
        self.center = center + img * box
        self.image = img

Some systems are getting hung up in this while loop.

We can replace this with freud's get_images and wrap.

JimmyRushing commented 2 years ago

Jank proposed that a possible snag here could be the "<" as opposed to "<=". This change didn't resolve the workflow breakdown. He also suggested that the [atom_ids] indexing should take place one line earlier for performance purposes. Nevertheless, I have created a PR with new logic that incorporates the freud functionalities suggested by jenny above.