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

Fix to the parameter transformation functionality #42

Closed yonatank93 closed 2 years ago

yonatank93 commented 2 years ago

List of changes:

mjwen commented 2 years ago

@yonatank93 thank for the PR! We may not want to touch the OptimizingParameters class. Can you please enable Allow edits from maintainers for me to make some adjustments?

yonatank93 commented 2 years ago

@mjwen I enabled Allow edits from maintainers. Let me know if this is not working.

mjwen commented 2 years ago

The functionality to deal with parameter transformation has all been moved to the Model class, leaving OptimizingParams free. Expect the below behaviors:

@yonatank93 Can you test this out and let me know whether this solves your problem?

yonatank93 commented 2 years ago

In OptimizingParams().echo_opt_params(), would it be useful to add a line in the heading "Parameters in transformed space.", like in model.echo_model_params()? Or to the least, we should mention in the documentation for this method that the printed values are in the transformed space.

mjwen commented 2 years ago

Agreed! It's useful to let the user know that it's in the transformed space. Added.

Also, it would be helpful to add a tutorial example to demonstrate this. Can you convert your Jupyter notebook to an example as those listed here? Probably take a look at example_kim_SW_Si.py. The docstrings in the examples follow the sphinx-gallery syntax.

yonatank93 commented 2 years ago

Sure, I will do it later today. In the example, we want to show:

mjwen commented 2 years ago

Thanks!

I will merge and close this. Feel free to open a new PR for the tutorial.