-
Notifications
You must be signed in to change notification settings - Fork 16
CG utils: implement ClebschGordanProduct
and refactor DensityCorrelations
#318
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
Conversation
Here is a pre-built version of the code in this pull request: wheels.zip, you can install it locally by unzipping |
TensorCorrelator
and refactor DensityCorrelations
TensorCorrelator
and refactor DensityCorrelations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the new TensorCorrelator, but I'm still not convinced by having a single DensityCorrelations for both self- and cross-correlations.
I would split it into something like SelfCorrelator
and CrossCorrelator
, maybe renaming TensorCorrelator
to ClebschGordanProduct
?
python/rascaline/rascaline/utils/clebsch_gordan/_density_correlations.py
Outdated
Show resolved
Hide resolved
python/rascaline/rascaline/utils/clebsch_gordan/_density_correlations.py
Outdated
Show resolved
Hide resolved
python/rascaline/rascaline/utils/clebsch_gordan/_tensor_correlator.py
Outdated
Show resolved
Hide resolved
python/rascaline/rascaline/utils/clebsch_gordan/_tensor_correlator.py
Outdated
Show resolved
Hide resolved
python/rascaline/rascaline/utils/clebsch_gordan/_tensor_correlator.py
Outdated
Show resolved
Hide resolved
I somewhat agree with splitting And to be fair, thinking about it, I'm not sure There isn't a use case where doing density cross correlations is actually useful, because the building block is an arbitrary body order density auto-correlation, which is then correlated with a different density (or something else). |
So instead I think DensityCorrelations then collapses back to what it always was before, a convenient way to compute density auto-correlations to arbitrary correlation order. |
That sounds good to me! |
python/rascaline/rascaline/utils/clebsch_gordan/_density_correlations.py
Outdated
Show resolved
Hide resolved
python/rascaline/rascaline/utils/clebsch_gordan/_density_correlations.py
Show resolved
Hide resolved
python/rascaline/rascaline/utils/clebsch_gordan/_density_correlations.py
Outdated
Show resolved
Hide resolved
python/rascaline/rascaline/utils/clebsch_gordan/_density_correlations.py
Outdated
Show resolved
Hide resolved
python/rascaline/rascaline/utils/clebsch_gordan/_tensor_correlator.py
Outdated
Show resolved
Hide resolved
Co-Authored-By: Guillaume Fraux <[email protected]>
TensorCorrelator
and refactor DensityCorrelations
ClebschGordanProduct
and refactor DensityCorrelations
Implements a general
TensorCorrelator
class and refactorsDensityCorrelations
to be built on top of this. The former allows the CG tensor product of two arbitrary TensorMaps (under some constraints, mostly to do with the metadata structure as explained in the class documentation), while the latter is a convenience class specifically for computing iterative density auto-correlations.Closes #312
📚 Documentation preview 📚: https://rascaline--318.org.readthedocs.build/en/318/