Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mcaroba opened this issue Oct 15, 2023 · 2 comments

Comments

@mcaroba
Copy link
Contributor

mcaroba commented Oct 15, 2023

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
Copy link
Member

albapa commented Oct 15, 2023

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
Copy link
Contributor Author

mcaroba commented Oct 16, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants