libAtoms / QUIP

libAtoms/QUIP molecular dynamics framework: https://libatoms.github.io
347 stars 121 forks source link

Added better compatibility for soap_turbo definitions and add_species=T support #621

Open mcaroba opened 11 months ago

mcaroba commented 11 months ago

I have significantly improved (simplified) the use of soap_turbo descriptors. Now one can use implicit definition of hyperparameters, e.g., atom_sigma=scalar automatically sets the atom_sigma_r and atom_sigma_t arrays. Also, some of the more obscure hyperparameters don't need to be defined, e.g., atom_sigma_r/t_scaling, which now are set to zero by default and for which one can also use a shortcut atom_sigma_scaling=scalar. There is compatibility with hypers inherited from soap: n_max, cutoff, cutoff_transition_width. I will add a comprehensive list as soon as I can.

I would appreciate if people can test the implementation and the use of add_species=T, which was also previously missing and I have added, especially those who were already using soap_turbo before.

Finally, @albapa the automated tests are failing, but they were already failing for the previous commit, so I'm not sure if that has anything to do with my updates. I have done some comprehensive tests and the code definitely compiles (at least with gfortran), both gap_fit binary and the quippy parts. All the tests I did for one and two species fits trying all the combinations of string shortcuts I could think of also seem to work.

albapa commented 11 months ago

Maybe it's worth looking at what causes the tests to fail. It's possible there is some more numerical discrepancy which is fine - just commit the new benchmarks or adjust the tolerance. I can also take a look if needed.

mcaroba commented 11 months ago

There's something about pip and sphinx: https://github.com/libAtoms/GAP/actions/runs/6525579900/job/17718407833. I think it's the same issue with the tests as was already there with your earlier commit. I don't know where to even start looking at this, I'm not familiar with that part of the code at all.

Once you give me thumbs up, I will update the GAP submodule in QUIP/src to add the new changes to the soap_turbo string handling.