search

lumol-org / lumol

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

Morse Potential implementation #192

Closed dd10-e closed 7 years ago

dd10-e commented 7 years ago

So sorry guys, i had to recreate the same PR because I re-fork the project : PR#191

I have listened your feedbacks and i think it's now correct =)

dd10-e commented 7 years ago

WIP

dd10-e commented 7 years ago

For documentation, i don't understand exactly what i must put in .toml file ?

g-bauer commented 7 years ago

Do you mean the ```toml part within the potential documentation?

You should provide an example of how the user can use the potential from the input, e.g. (I am making values up)

[[pairs]]
atoms = ["A", "B"]
# Additional parameters here are 'depth', 'width' and 'r0'.
MorsePotential = {depth= "40 kJ/mol", width = "3.0 A", r0 = "4.0 A"}

I'm not sure about the units here :) Does that answer your question?

dd10-e commented 7 years ago

mm..i'm going to study tests 😄

g-bauer commented 7 years ago

mm..i'm going to study tests :smile:

Do you need help with the tests?

dd10-e commented 7 years ago

I will read the documentation and understand more myself for now, thank you. =)

dd10-e commented 7 years ago

I know that the tests are not good, someone have an idea of ​​the problem? (I don't think that the formula is erroneous)

Luthaf commented 7 years ago

You should be able to run the tests locally with cargo test - p lumol-core, so that you don't need to push to Travis everytime =)

dd10-e commented 7 years ago

So sorry guys, I made several gourdes, i think that all run correctly now =)

g-bauer commented 7 years ago

Looks good to me!

Should we also include the input tests within this PR? I am wondering, is the width parameter something like the force constant of the harmonic potential (controlling the width)? If so, should its unit be that of an inverted length?

Luthaf commented 7 years ago

So, with @EloD10 permission, I took over this PR to finish it.

I squashed his commits together and made a few changes, mainly reverting width to a, because a width in inverted length would be a bit strange as @g-bauer pointed out =)

@g-bauer, I'll let your merge this is tests passes and everything is ok for you!

@EloD10, I forced pushed to your fork to update the PR. You can run the following commands to update any local branch:

git fetch  # assuming origin is https://github.com/EloD10/lumol
git checkout dev
git reset --hard origin/dev
g-bauer commented 7 years ago

Merged. :champagne: @EloD10 Thanks for the PR.