openkim / kliff

KIM-based Learning-Integrated Fitting Framework for interatomic potentials.
https://kliff.readthedocs.io
GNU Lesser General Public License v2.1
34 stars 20 forks source link

First changes for `Parameter` class #132

Closed ipcamit closed 10 months ago

ipcamit commented 11 months ago

Summary

First major commit toward decoupling Parameters from models

Deleted tests

TODO

There is certain level of duplication in functionality. Specially between set_opt_param and set_param_mutable. set_param_mutable is supposed to mark the parameters which can mutate during training, while their values can be set in models using set_opt_param.

Originally set_opt_param did both of these tasks so it was fine. This is little confusing now as I guess in current framework more appropriate naming should be set_param_value etc. which would clash with update model params function.

So would try to comeup with more coherent API in future. Hence also holding on modifying UQ stuff till this interface is more mature.

Next up

mjwen commented 11 months ago

I guess this is not ready for review yet, right? If ready, ping me and I will take a look.

ipcamit commented 11 months ago

Sure. I was holding out for libdescriptor as it would be used in next transformation module of kliff. Now that I have SOAP implemented there, I will write to you and yonathan again for discussing design of new transformation module to contain all possible transforms. Afterwards we can start merging the changes

ipcamit commented 10 months ago

Will raise new pull request with just dataset class