Skip to content

Comments

Feat/n center overlap week1 week2#232

Open
kir943 wants to merge 7 commits intotheochem:masterfrom
kir943:feat/n-center-overlap-week1-week2
Open

Feat/n center overlap week1 week2#232
kir943 wants to merge 7 commits intotheochem:masterfrom
kir943:feat/n-center-overlap-week1-week2

Conversation

@kir943
Copy link

@kir943 kir943 commented Feb 14, 2026

Summary

  • Implemented general N-center Gaussian overlap integral

    $$ S = \int \prod_{i=1}^{N} \phi_i(\mathbf{r}) , d\mathbf{r} $$

  • Generalized existing 2-center Gaussian product theorem logic to N centers

  • Added contraction screening via upper-bound estimation

  • Added sparse tensor construction for efficiency

  • Verified reduction to existing 2-center overlap implementation

  • Added validation tests for primitive (s, px, py, pz) cases

  • Added symmetry and stress tests for higher-order cases (N ≥ 2)

  • No regressions in existing test suite

How To Test

# Run overlap validation tests
pytest test_overlap.py -v
pytest test_overlap_n.py -v

# Run N-engine stress tests
pytest test_n_engine_stress.py -v

# Run full test suite (ensure no regressions)
pytest tests/ --timeout=120
image image image

Existing tests: Not broken
291 passed, 279 skipped, 1 xfailed, 4 warnings in 71.02s (0:01:11)

Scope of PR

  1. Implements a general N-center Gaussian overlap engine
  2. Adds screening and sparse tensor support
  3. Includes validation against existing 2-center implementation
  4. Adds symmetry and stress tests
  5. Does not modify existing APIs or behavior

Checklist

  • Write a good description of what the PR does
  • Add tests for each unit of code added (engine + validation + stress tests)
  • Update documentation (if required)
  • Squash commits that can be grouped together
  • Rebase onto master
  • Run full test suite (no regressions)

Type of Changes

Type
🐛 Bug fix
✨ New feature
🔨 Refactoring
📜 Docs

Copy link
Collaborator

@msricher msricher left a comment

Choose a reason for hiding this comment

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

I'm happy with the code quality and the completeness of the tests for this point in the project. I'm not sure if we should be monitoring the performance at this point or what the goals are there (@marco-2023 ?).

Just make sure to move the test_*.py files to the tests/ directory.

@kir943
Copy link
Author

kir943 commented Feb 20, 2026

Thank you for the review and your feedback @msricher !

I’ve moved the overlap test files to the tests/ directory as suggested and cleaned up the PR so it only contains the Week 1–2 overlap implementation. All checks are now passing.

Please let me know if any further changes are needed. I’ll also be continuing with the Week 3 tasks.

@kir943
Copy link
Author

kir943 commented Feb 22, 2026

Hi @msricher and @PaulWAyers,

Following the previous review and feedback, I have now completed additional validation of the arbitrary-order overlap implementation.

Specifically:
• Added explicit tests for arbitrary N-center overlap (N=1 to N=6)
• Verified symmetry, stability, and correctness across multiple cases
• Ran the full GBasis test suite:
302 passed, 280 skipped, 1 xfailed —( no regressions introduced)

All CI checks are passing successfully.
Please let me know if you would like any further refinements at this stage, or if I should proceed with the Week 4 API design for intracule/extracule functions as planned.

Thank you!


for μ, ν, λ in product(range(n), repeat=3):
T[μ, ν, λ] = three_gaussian_overlap_primitive(
[basis[μ], basis[ν], basis[λ]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good implementation, just wondering if you can just use numpy indexing here for simplicity:

T[u, v, l] = three_gaussian_overlap_primitive(basis[(u, v, l),])

Copy link
Author

@kir943 kir943 Feb 22, 2026

Choose a reason for hiding this comment

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

Thank you @msricher for the suggestion!

You're right .This multi_overlap.py file was part of my Week 3 experimental work while exploring arbitrary-order overlap extensions. Since this PR is focused on the Week 1–2 N-center overlap engine implementation, I removed that file to keep the scope clean and aligned with the current milestone.

The finalized implementation in this PR is in gbasis/integrals/overlap_n.py, along with its associated validation and stress tests.

I appreciate the numpy indexing suggestion and will keep it in mind as I continue developing the Week 3 functionality in a separate PR.

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