soap_turbo may not be thread safe

[bernstei]

The use of function-local save variables breaks thread safety, and soap_turbo uses it for W, S, and multiplicity_array in soap_turbo.f90 (line 71).

Has anyone thought about this issue yet? Can anyone explain what those variables are, and why they are save?

[mcaroba]

I have indeed not thought about thread safety. The W and S variables are built during orthogonality construction and are save to avoid having to recompute the radial basis coefficients every time the subroutine is invoked. There is a recompute_basis variable (also save) which is used to recompute the basis whenever subsequent calls to the subroutine use a different basis set (or the basis has not been previously computed). I think this might be enough to ensure thread safety, but I'm not 100% sure.

(S is the matrix of overlap coefficients of the original [non orthonormal] basis, and W is the matrix of linear coefficients to express the orthonormal basis in terms of the original basis)

[bernstei]

Thanks. I'll think about it as well.

[bernstei]

Are these arrays only dependent on properties of the GAP itself, or do they sometimes need to be recomputed based on something which is only known at compute time, e.g. the specific composition of the configuration?

I.e. if I knew I was only using one GAP+SOAP_turbo potential, could I do all the pre-computation once and count on it not changing during different compute calls?

[I'm thinking of refactoring a bit of it to create a soap_turbo object with a distinct Initialise call, like the potentials, to store this information so there can be two co-existing soap_turbo-based GAPs loaded at the same time, as in what could happen in multithreaded operation]

[mcaroba]

These arrays depend only on:

• alpha_max for the poly3 basis, so it can actually be tabulated. In fact I recently added a version of this basis with tabulated coefficients.
• alpha_max and atom_sigma_r for the poly3gauss basis.

They only need to be recomputed if these details change between calls, which will not happen unless the descriptor definition changes.

[bernstei]

OK - I may try to make a PR that will create some sort of soap_turbo object that's explicitly initialized, like the potentials, so it can be thread safe. I can even have the argument be optional, so it can fall back to the current behavior and no code that uses soap_turbo has to change unless it wants to be thread safe.

[mcaroba]

OK, sounds good!