Use switching function for Coulomb prior

torchmd / torchmd-net

Training neural network potentials

MIT License

335 stars 75 forks source link

Use switching function for Coulomb prior #287

Closed peastman closed 9 months ago

peastman commented 9 months ago

Originally I scaled the Coulomb prior by $\text{erfc}(\alpha r)$ to reduce its effect at short distances. It turned out this didn't work well because it was still too short ranged. Even at distances of only 1-2 A there was already a large component of Coulomb energy. I changed it to a cosine switching function which works much better.

peastman commented 9 months ago

I think the test failure was caused by #286. It's also failing on the main branch.

stefdoerr commented 9 months ago

Fixed in main now

RaulPPelaez commented 9 months ago

Looks good to me. This invalidates checkpoints using the Coulomb prior. I get that it is for the better but it is starting to happen a lot. We really need to cook versioning into the checkpoints...

RaulPPelaez commented 9 months ago

I added the versioning functionality in #288

peastman commented 9 months ago

I don't think anyone has ever used the Coulomb prior. It just didn't work very well. I wouldn't worry about breaking compatibility with it.

peastman commented 9 months ago

Tests are passing now.