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

Added cg_utils to Rascaline #185

Closed
wants to merge 4 commits into from
Closed

Conversation

ecignoni
Copy link
Contributor

@ecignoni ecignoni commented May 23, 2023


📚 Documentation preview 📚: https://rascaline--185.org.readthedocs.build/en/185/

@Luthaf
Copy link
Owner

Luthaf commented May 23, 2023

This is missing a test to ensure the code actually does what we think it do!

@Luthaf
Copy link
Owner

Luthaf commented May 23, 2023

Linter is not happy, you can run it locally with tox -e lint and fix most of the issues with tox -e format

@agoscinski
Copy link
Collaborator

Writing down TODOs because I will probably not touch it for a week

TODOs for this PR:

TODOs in following PRs:

@agoscinski
Copy link
Collaborator

So there is a more documented version in librascal https://github.com/lab-cosmo/librascal/blob/master/bindings/rascal/utils/cg_utils.py Also with two very good notebook examples. nice_demo and equivariant_demo. I will try to merge these two into the current version.

I still need more time to understand the code, its quite complicated. I think we can completely get rid of this combinations_string by enforcing an order of the dimension in the array. The flexibility by the argument makes it harder to understand and to use. From the examples I cannot really gather a real advantage for this flexibility.

@Luthaf
Copy link
Owner

Luthaf commented Sep 28, 2023

Closing in favor of #237

@Luthaf Luthaf closed this Sep 28, 2023
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.

4 participants