Skip to content

Conversation

@Roy-Kid
Copy link
Collaborator

@Roy-Kid Roy-Kid commented Mar 2, 2022

fix OOM bug;
api.py:302: add default polarity;
api.py:420: move pol&tholes into if pol:;
api.py:396: remove U_ind from params, access it directly from pme_force

Copy link
Collaborator

@KuangYu KuangYu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change the default setting of ethresh for both admp dispersion and electrostatics? Also, it may be good to start thinking about jit and vmap all the classical potentials (maybe do it in the next pr).

dmff/api.py Outdated
self._jaxPotential = None
self.types = []
self.ethresh = 1.0e-5
self.ethresh = 1e-5
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change it to 5e-4, keep consistent with electrostatic PME

dmff/api.py Outdated
# self._input_params = defaultDict(list)
self._jaxPotential = None
self.types = []
self.ethresh = 1.0e-5
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to 5e-4


def generate_get_energy(self):
def get_energy(positions, box, pairs, k, theta0):
p1 = positions[self.p1idx]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a remind that there are distribute_xx functions in admp.pairwise module (maybe it is not the most appropriate place to put these functions ...), which were designed to do such parameter expansions. These functions are jitted and vmapped, and my tests show that they can be noticeably faster. It's okay for now, but in the next stage we may want to think about jit and vmap all classical potentials.

@KuangYu KuangYu merged commit f7ae72b into deepmodeling:devel Mar 3, 2022
Roy-Kid referenced this pull request in Roy-Kid/DMFF Mar 19, 2022
KuangYu pushed a commit that referenced this pull request Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants