lumol-org / lumol

Universal extensible molecular simulation engine
http://lumol.org/
BSD 3-Clause "New" or "Revised" License
184 stars 18 forks source link

Added WCA #229

Closed gjepson closed 6 years ago

gjepson commented 6 years ago

Added WCA. Starts on line 114 of functions.rs. All tests passed.

Luthaf commented 6 years ago

Thank you for sending the Pull-Request! 👍

The code looks good to me, could you add some tests for it? Look line 560 and following to get an idea about how they work. The idea is to compute the energy and force value by hand on some special values, and check that the code matches the compute value. Another good test is to check that computing the energy derivative by finite differences matches the force.

Travis tests are not passing on OS X, but the failure is not related to your code. I think it comes from Travis updating the version of OS X they use.


I don't know anything about WCA potentials, could you expand on why they is two separate parts for it (attractive and repulsive)? Do people actually need to use these term separately? Ping @g-bauer on this, you also seems to know the method.

gjepson commented 6 years ago

My mistake, I committed old code, forgetting to update my online repository from my local repository. WCA is defined for both a repulsive and attractive portion, however in computations the repulsive portion the only one used. I had written it as 2 separate functions until my adviser told me this, so I corrected it locally but forgot to update online. I will change that and then issue another pull request.

I will add the tests with the new code.

g-bauer commented 6 years ago

Hey, thanks for your contribution. If you want to use your potential as intermolecular non-bonded potential the next step would be to also implement the PairPotential trait. If you need help with this or if you want to be able to use your potential via our input files, please let me know.

gjepson commented 6 years ago

Looking through the documentation for pair potentials, I noticed that PairPotentials has user define the cutoff, but the thought for WCA is that the potential could define it. Is there an option to do allow for this, or could I program one in?

Luthaf commented 6 years ago

So, correct me if I'm wrong, but WCA want to always use a specific cutoff (2^1/6 \sigma) ?

The PairInteraction ask the user for a cutoff, but it is intended for the generic case of non-bonded interactions. In fact, WCA could be implemented purely in the current code as a Lennard-Jones potential with a cutoff of (2^1/6 \sigma), right ?

To answer your questions, there is currently no way for a potential to define a cutoff, but we could add one (adding a default method to the trait fn cutoff(self) -> Option<Cutoff>)

Maybe, we could also do this within the current framework, by having WCA potential only defined as a special case in the input parsing, generating a standard lennard-jones + cutoff.

gjepson commented 6 years ago

That's correct. I will implement it with the PairPotentials for now, and then consider implementing a method at a later date.