Skip to content

PR1 : implement analytical Coulomb potential for Gaussian densities#304

Open
Ao-chuba wants to merge 3 commits into
theochem:masterfrom
Ao-chuba:feature/coulomb-analytical-potentials
Open

PR1 : implement analytical Coulomb potential for Gaussian densities#304
Ao-chuba wants to merge 3 commits into
theochem:masterfrom
Ao-chuba:feature/coulomb-analytical-potentials

Conversation

@Ao-chuba
Copy link
Copy Markdown
Member

First inital PR for Improvement of the robustness of poisson equation .
This PR implements the core mathematical engine for evaluating the exact analytical electrostatic (Coulomb) potential of $s$-type and $p$-type radial Gaussian charge densities. The math is derived from the exact formulations and $r \to 0$ limits outlined in the course project reference (https://qchem.qc-edu.org/problems/HF_dft.html). This PR contains the standalone coulomb.py module and an isolated unit test suite validating math correctness across various distances and normalization schemes, establishing the foundation for the subsequent robust Poisson solver scaffolding.
related to issue -> #288

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new standalone coulomb module providing exact analytical Coulomb potential evaluations for s-type and p-type radial Gaussian charge densities, together with a higher-level coulomb_potential aggregator that sums contributions from a set of centered Gaussians. This is the first building block towards a more robust Poisson solver (issue #288).

Changes:

  • New src/grid/coulomb.py with coulomb_gaussian_s, coulomb_gaussian_p, and coulomb_potential, including correct handling of the r → 0 limit for both normalized and unnormalized conventions.
  • New src/grid/tests/test_coulomb.py exercising the analytical formulas at r=0 and at non-zero radii for both normalizations, plus an aggregate s-type-only consistency test against a hand-rolled reference.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/grid/coulomb.py New module implementing analytical Coulomb potentials of s- and p-type Gaussian densities and a multi-Gaussian aggregator.
src/grid/tests/test_coulomb.py New unit tests validating the s/p formulas at the origin and at non-zero radii, and an aggregate s-only check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/grid/coulomb.py Outdated
Comment thread src/grid/coulomb.py
Comment thread src/grid/coulomb.py Outdated
Comment thread src/grid/coulomb.py Outdated
@Ao-chuba
Copy link
Copy Markdown
Member Author

as per copilot suggestions i took into consideration for some of its inputs:

  • Added validation inside coulomb_potential ensuring that coeffs_p, expons_p, and centers_p are either all provided or all omitted to prevent silent dropping of p-type density.
  • Added checks to ensure that the coefficients, exponents, and centers have the same length. This prevents silent truncation of Gaussians due to Python's zip behavior.
  • Added checks to ensure alpha > 0 and beta > 0. This prevents invalid math operations (like taking np.sqrt of negative numbers or division by zero).
  • changed number 1e-12 to a named module-level constant (_R_ZERO_THRESHOLD) to avoid duplication and improve readability.

Comment thread src/grid/coulomb.py Outdated
Comment thread src/grid/coulomb.py Outdated
Comment thread src/grid/tests/test_coulomb.py Outdated
Copy link
Copy Markdown
Collaborator

@marco-2023 marco-2023 left a comment

Choose a reason for hiding this comment

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

Nice work! I think this is a good progress.
Please simplify the functions.
Homogenize the tests

I think this module should not be in the root of the package, but we can leave that for another moment.

@Ao-chuba
Copy link
Copy Markdown
Member Author

Thanks for the input sir, i updated my code as per the suggestions. please do check and let me know if any other things can be improved too.

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.

3 participants