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

Bugfix: Overlap matrix had extra normalization #406

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kvhuguenin
Copy link
Contributor

Fixes the wrong orthonormalization in rascal for GTO basis.

The code was mixing two conventions for the GTOs.
The overlap matrix was defined using the normalized GTOs, while the rest of the code is using the primitive GTOs (just the r^n times Gaussian part). Now, the coefficients obtained from librascal agree with those from "exact calculations" and those from pyLODE up to a global factor.

The original code was trying to take the extra normalization into account by multiplying the expansion coefficients again with the same normalization factors. Most likely, there was something wrong in the execution of these steps. We are trying to understand this in more detail, but for the time being, the new coefficients agree with the approaches mentioned above and seem to be correct.

Still TODO:

  • Fix some errors associated with this in the documentation.
  • Clean up code to remove code fragments belonging to redundant normalization steps

@Luthaf
Copy link
Contributor

Luthaf commented May 9, 2022

We will have to also change the regression tests data and the python implementation of GTO in https://github.com/lab-cosmo/librascal/blob/fed28b910d9ba6f8549c935aaf67bdc14051d511/bindings/rascal/utils/radial_basis.py.

@Luthaf
Copy link
Contributor

Luthaf commented Jul 1, 2022

@kvhuguenin should we close this?

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

Successfully merging this pull request may close these issues.

2 participants