ContactEngineering / Adhesion

Cohesive zone models and intermolecular interactions for contact calculations
https://contactengineering.github.io/Adhesion/
MIT License
5 stars 2 forks source link

API: Interactions should return potential, gradient, curvature instead of potential, force, curvature #3

Closed sannant closed 2 years ago

sannant commented 4 years ago

I don't like to change the API, but returning the forces (pressures actually) instead of the gradient is not the most intuitive thing.

Very misleading is that the forces are called dV inside the code.

At least we should more explicitely state that we return minus the gradient.

r""" Evaluates the potential and its derivatives without cutoffs or offsets. These have been collected in a single method to reuse the computated LJ terms for efficiency V(g) = -gamma0e^(-g(r)/rho) V'(g) = (gamma0/rho)e^(-g(r)/rho) V''(g) = -(gamma0/r_ho^2)*e^(-g(r)/rho)

        Keyword Arguments:
        r      -- array of distances
        pot    -- (default True) if true, returns potential energy
        forces -- (default False) if true, returns forces
        curb   -- (default False) if true, returns second derivative

"""

@pastewka what is your opinion ?

pastewka commented 4 years ago

I would change it to something simpler, maybe just the gradient. The sign does not really matter as it depends on whether you compute forces on top or bottom surface. Please change this to just the derivatives (gradient).

sannant commented 4 years ago

ok, also pot, forces, curb arguments are not very good names. I would rather call them evaluate_potential, evaluate_gradent, evaluate_curvature

pastewka commented 4 years ago

I would drop the evaluate prefix

sannant commented 4 years ago

Ok. I am thinking about optionaly using these arguments to directly fill an array, as proposed here https://github.com/pastewka/PyCo/issues/137#issuecomment-611552996

There will then be two ways to call the potential:

potential = np.zeros(nb_grid_pts)
gradient = np.zeros(nb_grid_pts)

mask = gap < cutoff
LJ82.evaluate(potential=potential, gradient=gradient , mask=mask)

or

potential[mask], gradient[mask],_ = LJ82.evaluate(potential=True, gradient=True, mask=mask)

The second interface I would only implement in evaluate, and not in internal methods like "naive_pot"

sannant commented 4 years ago

Will be closed by bd859cf1