Utilities for the optimal radial basis calculation

[ceriottm]

Utility functions and one-line helper to compute optimal radial basis for a dataset

Rationale and detailed description of the changes:
The optimal radial basis calculation requires a lot of manual steps. This PR creates a handful of clean functions to perform the different steps, as well as a simple wrapper that adds projection matrices to a template dictionary of hyperparameters.

Tasks before review:

• Add tests, examples, and complete documentation of any added functionality
  • Make sure the autogenerated documentation appears in Sphinx and is
    formatted correctly (ask @max-veit if you need help with this task).
  • Write additional documentation in the Sphinx (not docstrings!) to
    explain the feature and its usage in plain English
  • Make sure the examples run (!) and produce the expected output
• Run make lint on the project, ensure it passes
  • Run make pretty-cpp and make pretty-python, check that the
    auto-formatting is sensible
  • Review variable names, make sure they're descriptive (ask a friend)
  • Review all TODOs, convert to issues wherever possible
  • Make sure draft, in-progress, and commented-out code is moved to its
    own branch
• If committing any code in Jupyter/IPython notebooks, install and run nbstripout
• Merge with master and resolve any conflicts

[agoscinski]

Looks already good.

• needs some simple tests for the new utils function
• I would just adapt the notebook a bit, especially the last parts are sparse in explanations (also I would like to put my visual example for the radial spectrum density back)

I would just do a commit with these changes, and you can give me a comment on the changes, but you can also do it if you want

I think the example notebook has to be git mv so the changes are tracked, but this can be done in the rebase.

[agoscinski]

This is here because zundel notebook is a notebook where the the output is important and a change in the output should be recorded by git, correct?
Then I would still do it in another PR, since you don't touch the notebook here. Or did you touch the notebook and did not commit it?

[max-veit]

It actually appears in another PR (#354) where the notebook is changed, so I'd leave it for there.

[agoscinski]

would remove the commented line then and add comment

# Symmetric orthogonalization matrix

[ceriottm]

I don't think these are ever computed on the C++ side. These are needed only to reconstruct real-space quantities, and they are used very often (e.g. by Ben) so I felt they would fit within the scope of a "radial basis utilities" PR. Much like the other utilities in this PR, the point is avoiding to reimplement the same little utilities to work with radial bases.

…

On Wed, 2 Jun 2021 at 12:16, Guillaume Fraux ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In bindings/rascal/utils/radial_basis.py <#358 (comment)>: > @@ -0,0 +1,415 @@ +""" +A collection of utility functions to manipulate the radial basis. +""" + +from scipy.special import legendre, gamma +from copy import deepcopy +import numpy as np +from ..representations.spherical_expansion import SphericalExpansion + + +def radial_basis_functions_dvr( Why is this function (and radial_basis_functions_gto) needed? It seems it is only used in an example to look at the radial function after optimization, right? I would rather move the function to the example notebook then, rather than having it inside the rascal package. Otherwise, this is a duplicated implementation with code in C++, and we need to make sure they stay in sync. At the very least, there should be a big WARNING comment both here and in the C++ implementation to ensure they are kept in sync. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#358 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIREZ6IFGECHHMA7ZOGTGDTQYAHJANCNFSM45XKA67Q> .

[max-veit]

Might this be a good notebook to include in the examples page? If so, you can link the file into the docs/source/examples/ directory and then add it here: https://github.com/cosmo-epfl/librascal/blob/4e576ae7b9d3740715ab1910def5e1a15ffd1268/docs/source/examples/examples.rst#L09-L12
Then try compiling the docs locally and see if it produces any errors :)

[ceriottm]

Works! Will push with next commit

[agoscinski]

@ceriottm does it really work? If I compile it locally I don't see the radial optimizations notebook. I find that weird because I could not find zundel_i-PI.ipynb added anywhere else than here, so I am not sure why this is

[ceriottm]

OK I think I implemented all the changes you guys asked. PLS have a second round if you want to.

[agoscinski]

Hello you need to add your new tests to tests/python_binding_test.py by adding a entry

from python_utils_test import TestOptimalRadialBasis

Then you get some test error (reminder ctest -R python to only run python test), I guess it will be simple to fix

[agoscinski]

I am okay with branch. It only needs to be checked if the problem that the notebook is now shown in the docs examples is only a local problem for me.
And would squash commits to one.

[ceriottm]

I am okay with branch. It only needs to be checked if the problem that the notebook is now shown in the docs examples is only a local problem for me.
And would squash commits to one.

what do you mean the problem with the notebook? it shows correctly in my local documentation

[agoscinski]

I don't see the notebook it in the examples at all in the doc page, but it seems to be only on my side. So I am okay with the branch, just squash and merge to one commit or rebase in some other way