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

Update basisset to use numpy over jax.numpy #126

Merged
merged 3 commits into from
Oct 17, 2023
Merged

Update basisset to use numpy over jax.numpy #126

merged 3 commits into from
Oct 17, 2023

Conversation

hatemhelal
Copy link
Contributor

This updates the basisset factory function to prefer directly using numpy arrays over jax.numpy. Since a Basis consists of a number of constants we can directly manipulate those using numpy functions. For example, with this PR the Primitives are also normalised using numpy functions.

@hatemhelal hatemhelal requested a review from awf October 15, 2023 13:17
@hatemhelal hatemhelal self-assigned this Oct 15, 2023
Copy link
Collaborator

@awf awf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might we ever need to compute a gradient over the normalizing constant?

@hatemhelal
Copy link
Contributor Author

Might we ever need to compute a gradient over the normalizing constant?

The only case I can think of where we might need that is if we are optimising the basis set exponents (alphas). For the use cases of structure optimisation and molecular dynamics we would expect the normalising term to be a constant since it doesn't depend on the basis centers.

That said, I think since the norm term is an optional we could support the basis set exponent optimisation use case by having a different factory function that creates a Primitive with a differentiable norm term.

@hatemhelal hatemhelal merged commit 0e986f6 into main Oct 17, 2023
4 checks passed
@hatemhelal hatemhelal deleted the fix-jnp branch October 17, 2023 09:01
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